[PATCH 0/5] virtio-blk: Deprecate silent truncation of serial
QEMU has allowed the user to specify serials of arbitrary length for virtio-blk devices, even though the virtio-blk protocol can only communicate 20 characters of the serial to the guest. This means that a longer serial number on the command line is silently truncated to 20 characters from the guest perspective. As a first step to fix this, deprecate the silent truncation and turn it into a warning; we can later make this a hard error. While touching GET_ID, let's make it more spec compliant and clean up the internal state. Another part is planned that will actually enable users to use longer serials and additional device identifiers. This will need spec changes and would take more time than we have until the QEMU 11.2 freeze anyway, so I'll address this later. For now, the important thing is to get the deprecation period started. Kevin Wolf (5): virtio-blk: Implement full NUL padding for GET_ID virtio-blk-test: Add a basic GET_ID test case virtio-blk: Deprecate silent truncation of serial virtio-blk: Represent truncated serial in qdev property virtio-blk-test: Test truncated serial docs/about/deprecated.rst | 13 +++++ hw/block/virtio-blk.c | 24 +++++++-- tests/qtest/virtio-blk-test.c | 92 +++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 4 deletions(-) -- 2.56.0
The spec requires not only NUL termination of the device ID string, but NUL padding: VIRTIO_BLK_T_GET_ID requests fetch the device ID string from the device into data. The device ID string is a NUL-padded ASCII string up to 20 bytes long. If the string is 20 bytes long then there is no NUL terminator. Zero out all of the remaining bytes in the request buffer. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- hw/block/virtio-blk.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 536d71a91ad..3f236f22d57 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1011,14 +1011,18 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) case VIRTIO_BLK_T_GET_ID: { /* - * NB: per existing s/n string convention the string is + * The spec requires NUL-padding for the output. The string is * terminated by '\0' only when shorter than buffer. */ const char *serial = s->conf.serial ? s->conf.serial : ""; - size_t size = MIN(strlen(serial) + 1, - MIN(iov_size(in_iov, in_num), - VIRTIO_BLK_ID_BYTES)); + size_t req_size = MIN(iov_size(in_iov, in_num), VIRTIO_BLK_ID_BYTES); + size_t size = MIN(strlen(serial), req_size); + size_t padding = req_size - size; + iov_from_buf(in_iov, in_num, 0, serial, size); + if (padding) { + iov_memset(in_iov, in_num, size, 0, padding); + } virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); g_free(req); break; -- 2.56.0
On Thu, Oct 29, 2026 at 12:23:56AM +0100, Kevin Wolf wrote:
The spec requires not only NUL termination of the device ID string, but NUL padding:
VIRTIO_BLK_T_GET_ID requests fetch the device ID string from the device into data. The device ID string is a NUL-padded ASCII string up to 20 bytes long. If the string is 20 bytes long then there is no NUL terminator.
Zero out all of the remaining bytes in the request buffer.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- hw/block/virtio-blk.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
This covers the simple common case with no surprises: A serial that is shorter than 20 characters and can fit in the request buffer. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qtest/virtio-blk-test.c | 69 +++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c index 98c906ebb4a..14fadcb4009 100644 --- a/tests/qtest/virtio-blk-test.c +++ b/tests/qtest/virtio-blk-test.c @@ -95,6 +95,8 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d, g_assert_cmpuint(data_size % sizeof(struct virtio_blk_discard_write_zeroes), ==, 0); break; + case VIRTIO_BLK_T_GET_ID: + break; default: g_assert_cmpuint(data_size, ==, 0); } @@ -771,6 +773,68 @@ static void resize(void *obj, void *data, QGuestAllocator *t_alloc) } +static QVirtQueue *get_id_setup(QVirtioDevice *dev, QGuestAllocator *t_alloc) +{ + QVirtQueue *vq; + uint64_t features = qvirtio_get_features(dev); + + features = features & (1ull << VIRTIO_F_VERSION_1); + qvirtio_set_features(dev, features); + + vq = qvirtqueue_setup(dev, t_alloc, 0); + qvirtio_set_driver_ok(dev); + return vq; +} + +static uint8_t get_id_request(QVirtioDevice *dev, QVirtQueue *vq, + char *buf, size_t buf_len, + QGuestAllocator *t_alloc) +{ + QTestState *qts = global_qtest; + uint64_t req_addr; + uint32_t free_head; + uint8_t status; + + QVirtioBlkReq req = { + .type = VIRTIO_BLK_T_GET_ID, + .data = buf, + }; + + req_addr = virtio_blk_request(t_alloc, dev, &req, buf_len); + + memset(req.data, 0xa5, buf_len); + memwrite(req_addr + 16, req.data, buf_len); + + free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true); + qvirtqueue_add(qts, vq, req_addr + 16, buf_len, true, true); + qvirtqueue_add(qts, vq, req_addr + 16 + buf_len, 1, true, false); + + qvirtqueue_kick(qts, dev, vq, free_head); + + qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL, + QVIRTIO_BLK_TIMEOUT_US); + status = readb(req_addr + 16 + buf_len); + memread(req_addr + 16, buf, buf_len); + return status; +} + +static void get_id_serial_short(void *obj, void *data, QGuestAllocator *t_alloc) +{ + QVirtioBlk *blk_if = obj; + QVirtioDevice *dev = blk_if->vdev; + QVirtQueue *vq; + uint8_t status; + const char expected[20] = "abc"; + char buf[20]; + + vq = get_id_setup(dev, t_alloc); + status = get_id_request(dev, vq, buf, 20, t_alloc); + g_assert_cmpint(status, ==, 0); + g_assert_cmpmem(buf, 20, expected, sizeof(expected)); + + qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc); +} + static void *virtio_blk_test_setup(GString *cmd_line, void *arg) { char *tmp_path = drive_create(); @@ -802,6 +866,11 @@ static void register_virtio_blk_test(void) qos_add_test("nxvirtq", "virtio-blk-pci", test_nonexistent_virtqueue, &opts); qos_add_test("hotplug", "virtio-blk-pci", pci_hotplug, &opts); + + /* serial number tests */ + opts.edge.extra_device_opts = "serial=abc"; + qos_add_test("get_id/serial_short", "virtio-blk", + get_id_serial_short, &opts); } libqos_init(register_virtio_blk_test); -- 2.56.0
On Thu, Oct 29, 2026 at 12:23:57AM +0100, Kevin Wolf wrote:
@@ -802,6 +866,11 @@ static void register_virtio_blk_test(void) qos_add_test("nxvirtq", "virtio-blk-pci", test_nonexistent_virtqueue, &opts); qos_add_test("hotplug", "virtio-blk-pci", pci_hotplug, &opts); + + /* serial number tests */ + opts.edge.extra_device_opts = "serial=abc"; + qos_add_test("get_id/serial_short", "virtio-blk", + get_id_serial_short, &opts);
The edge case where the serial string is 20 ASCII characters long (no NUL padding) is worth testing now that you're adding the infrastructure to do so.
On Thu, Oct 29, 2026 at 12:23:57AM +0100, Kevin Wolf wrote:
+static uint8_t get_id_request(QVirtioDevice *dev, QVirtQueue *vq, + char *buf, size_t buf_len, + QGuestAllocator *t_alloc) +{ + QTestState *qts = global_qtest; + uint64_t req_addr; + uint32_t free_head; + uint8_t status; + + QVirtioBlkReq req = { + .type = VIRTIO_BLK_T_GET_ID, + .data = buf, + }; + + req_addr = virtio_blk_request(t_alloc, dev, &req, buf_len); + + memset(req.data, 0xa5, buf_len); + memwrite(req_addr + 16, req.data, buf_len); + + free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true); + qvirtqueue_add(qts, vq, req_addr + 16, buf_len, true, true); + qvirtqueue_add(qts, vq, req_addr + 16 + buf_len, 1, true, false); + + qvirtqueue_kick(qts, dev, vq, free_head); + + qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL, + QVIRTIO_BLK_TIMEOUT_US); + status = readb(req_addr + 16 + buf_len); + memread(req_addr + 16, buf, buf_len);
Missing guest_free(t_alloc, req_addr);
+ return status; +}
Commit a8686a9b, which made serial a qdev property instead of relying on -drive options, accidentally allowed specifying serial strings of arbitraty length while the virtio-blk protocol can only communicate 20 characters to the guest. This means that a longer serial number on the command line is silently truncated to 20 characters from the guest perspective. This is obviously bad behaviour. People are specifying longer serials in practice and guests will see something different than what was configured. Instead of silent truncation, this should be an error. Deprecate the silent truncation for now as a first step towards making it an error later. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- docs/about/deprecated.rst | 13 +++++++++++++ hw/block/virtio-blk.c | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index dab47b6ca38..0891bbc44de 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -362,6 +362,19 @@ the addition of volatile memory support, it is now necessary to distinguish between persistent and volatile memory backends. As such, memdev is deprecated in favor of persistent-memdev. +``-device virtio-blk,serial=more_than_20_characters`` (since 11.2) +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The ``virtio-blk`` device has a limitation of 20 characters for the serial +string. Accidentally, QEMU started accepting longer strings on the command line +in version 0.15.0, but it was still silently truncated to 20 characters for +guests. + +Specifying a longer serial than can be exposed to the guest is a configuration +error and deprecated; it will become a hard error in later versions. If the +truncation is intentional, explicitly specify the serial already in truncated +form on the command line instead. + Block device options '''''''''''''''''''' diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 3f236f22d57..eeeb405127c 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1849,6 +1849,12 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) return; } + if (conf->serial && strlen(conf->serial) > VIRTIO_BLK_ID_BYTES) { + warn_report("serial is longer than %d characters and will be truncated " + "for the guest. This configuration is deprecated.", + VIRTIO_BLK_ID_BYTES); + } + if (!blkconf_apply_backend_options(&conf->conf, !blk_supports_write_perm(conf->conf.blk), true, errp)) { -- 2.56.0
On Thu, Oct 29, 2026 at 12:23:58AM +0100, Kevin Wolf wrote:
Commit a8686a9b, which made serial a qdev property instead of relying on -drive options, accidentally allowed specifying serial strings of arbitraty length while the virtio-blk protocol can only communicate 20 characters to the guest. This means that a longer serial number on the command line is silently truncated to 20 characters from the guest perspective.
This is obviously bad behaviour. People are specifying longer serials in practice and guests will see something different than what was configured. Instead of silent truncation, this should be an error.
Deprecate the silent truncation for now as a first step towards making it an error later.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- docs/about/deprecated.rst | 13 +++++++++++++ hw/block/virtio-blk.c | 6 ++++++ 2 files changed, 19 insertions(+)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
If the guest can only see the first 20 characters of the serial, we shouldn't pretend to have a longer string in the 'serial' qdev property (which is visible through things like QOM or 'info qtree'). Truncate the serial already in the internal state of the device to make sure we don't mix both versions anywhere. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- hw/block/virtio-blk.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index eeeb405127c..cecd08a2f2e 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1850,9 +1850,15 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) } if (conf->serial && strlen(conf->serial) > VIRTIO_BLK_ID_BYTES) { + char *truncated; + warn_report("serial is longer than %d characters and will be truncated " "for the guest. This configuration is deprecated.", VIRTIO_BLK_ID_BYTES); + + truncated = g_strndup(conf->serial, VIRTIO_BLK_ID_BYTES); + g_free(conf->serial); + conf->serial = truncated; } if (!blkconf_apply_backend_options(&conf->conf, -- 2.56.0
On Thu, Oct 29, 2026 at 12:23:59AM +0100, Kevin Wolf wrote:
If the guest can only see the first 20 characters of the serial, we shouldn't pretend to have a longer string in the 'serial' qdev property (which is visible through things like QOM or 'info qtree').
Truncate the serial already in the internal state of the device to make sure we don't mix both versions anywhere.
This is a user-visible change that can break software relying on QEMU's serial string output. For example, imagine QEMU is launched with a serial string that is the UUID 22382676-e635-4dca-a1db-0eaf2e2e192a. Later on, a script fetches the serial from QEMU monitor output and uses it as the volume identifier in a distributed storage system (e.g. to fetch volume statistics for the guest's disk). That script will now break because QEMU no longer retains the full string. Let's hold off on this change until the deprecation grace period finishes.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- hw/block/virtio-blk.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index eeeb405127c..cecd08a2f2e 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1850,9 +1850,15 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) }
if (conf->serial && strlen(conf->serial) > VIRTIO_BLK_ID_BYTES) { + char *truncated; + warn_report("serial is longer than %d characters and will be truncated " "for the guest. This configuration is deprecated.", VIRTIO_BLK_ID_BYTES); + + truncated = g_strndup(conf->serial, VIRTIO_BLK_ID_BYTES); + g_free(conf->serial); + conf->serial = truncated; }
if (!blkconf_apply_backend_options(&conf->conf, -- 2.56.0
Am 29.10.2026 um 13:51 hat Stefan Hajnoczi geschrieben:
On Thu, Oct 29, 2026 at 12:23:59AM +0100, Kevin Wolf wrote:
If the guest can only see the first 20 characters of the serial, we shouldn't pretend to have a longer string in the 'serial' qdev property (which is visible through things like QOM or 'info qtree').
Truncate the serial already in the internal state of the device to make sure we don't mix both versions anywhere.
This is a user-visible change that can break software relying on QEMU's serial string output. For example, imagine QEMU is launched with a serial string that is the UUID 22382676-e635-4dca-a1db-0eaf2e2e192a. Later on, a script fetches the serial from QEMU monitor output and uses it as the volume identifier in a distributed storage system (e.g. to fetch volume statistics for the guest's disk). That script will now break because QEMU no longer retains the full string.
Do we know of any such management tools, or is the concern just out of an abundance of caution? It sounds fundamentally wrong to me to use the serial as opposed to the qdev id to identify devices.
Let's hold off on this change until the deprecation grace period finishes.
I can drop the patch for a v2 of this series (or just apply it without this patch if no other changes are necessary) as the value of the qdev property should be the only real effect at the moment. However, with the work that actually enables longer serials, having an internal state that doesn't match what the guest sees feels rather error prone. So I'd be happy to defer the change of the serial in the internal state to the 12.0 cycle, but probably not until after the deprecation period. Kevin
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- hw/block/virtio-blk.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index eeeb405127c..cecd08a2f2e 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1850,9 +1850,15 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) }
if (conf->serial && strlen(conf->serial) > VIRTIO_BLK_ID_BYTES) { + char *truncated; + warn_report("serial is longer than %d characters and will be truncated " "for the guest. This configuration is deprecated.", VIRTIO_BLK_ID_BYTES); + + truncated = g_strndup(conf->serial, VIRTIO_BLK_ID_BYTES); + g_free(conf->serial); + conf->serial = truncated; }
if (!blkconf_apply_backend_options(&conf->conf, -- 2.56.0
On Thu, Oct 29, 2026 at 02:16:38PM +0100, Kevin Wolf wrote:
Am 29.10.2026 um 13:51 hat Stefan Hajnoczi geschrieben:
On Thu, Oct 29, 2026 at 12:23:59AM +0100, Kevin Wolf wrote:
If the guest can only see the first 20 characters of the serial, we shouldn't pretend to have a longer string in the 'serial' qdev property (which is visible through things like QOM or 'info qtree').
Truncate the serial already in the internal state of the device to make sure we don't mix both versions anywhere.
This is a user-visible change that can break software relying on QEMU's serial string output. For example, imagine QEMU is launched with a serial string that is the UUID 22382676-e635-4dca-a1db-0eaf2e2e192a. Later on, a script fetches the serial from QEMU monitor output and uses it as the volume identifier in a distributed storage system (e.g. to fetch volume statistics for the guest's disk). That script will now break because QEMU no longer retains the full string.
Do we know of any such management tools, or is the concern just out of an abundance of caution?
It sounds fundamentally wrong to me to use the serial as opposed to the qdev id to identify devices.
Based on qemu-devel threads (e.g. tracking IOThread holders) it seems that people write management tools that query QEMU rather than maintaining their own state. While I don't know of a specific tool that uses the serial string, it doesn't seem far-fetched that someone might rely on this. I don't think we gain enough from changing the monitor output that it's worth risking breaking existing users.
Let's hold off on this change until the deprecation grace period finishes.
I can drop the patch for a v2 of this series (or just apply it without this patch if no other changes are necessary) as the value of the qdev property should be the only real effect at the moment.
However, with the work that actually enables longer serials, having an internal state that doesn't match what the guest sees feels rather error prone. So I'd be happy to defer the change of the serial in the internal state to the 12.0 cycle, but probably not until after the deprecation period.
I would still hold off until it is actually time to enforce the length limit according to the deprecation policy, but I guess there's an argument that once users can configure long strings, they should stop abusing the serial property. In any case, lets defer this patch for 11.2. Stefan
Only 20 bytes are written to the passed buffer and the returned string doesn't have a NUL terminator. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qtest/virtio-blk-test.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c index 14fadcb4009..99d785d9e1a 100644 --- a/tests/qtest/virtio-blk-test.c +++ b/tests/qtest/virtio-blk-test.c @@ -835,6 +835,25 @@ static void get_id_serial_short(void *obj, void *data, QGuestAllocator *t_alloc) qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc); } +static void get_id_serial_short_trunc(void *obj, void *data, + QGuestAllocator *t_alloc) +{ + QVirtioBlk *blk_if = obj; + QVirtioDevice *dev = blk_if->vdev; + QVirtQueue *vq; + uint8_t status; + /* Everything after the 20 character maximum is untouched, no '\0' */ + const char expected[25] = "01234567890123456789\xa5\xa5\xa5\xa5\xa5"; + char buf[25]; + + vq = get_id_setup(dev, t_alloc); + status = get_id_request(dev, vq, buf, 25, t_alloc); + g_assert_cmpint(status, ==, 0); + g_assert_cmpmem(buf, 25, expected, sizeof(expected)); + + qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc); +} + static void *virtio_blk_test_setup(GString *cmd_line, void *arg) { char *tmp_path = drive_create(); @@ -871,6 +890,10 @@ static void register_virtio_blk_test(void) opts.edge.extra_device_opts = "serial=abc"; qos_add_test("get_id/serial_short", "virtio-blk", get_id_serial_short, &opts); + + opts.edge.extra_device_opts = "serial=012345678901234567890123456789"; + qos_add_test("get_id/serial_short_trunc", "virtio-blk", + get_id_serial_short_trunc, &opts); } libqos_init(register_virtio_blk_test); -- 2.56.0
On Thu, Oct 29, 2026 at 12:24:00AM +0100, Kevin Wolf wrote:
Only 20 bytes are written to the passed buffer and the returned string doesn't have a NUL terminator.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qtest/virtio-blk-test.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c index 14fadcb4009..99d785d9e1a 100644 --- a/tests/qtest/virtio-blk-test.c +++ b/tests/qtest/virtio-blk-test.c @@ -835,6 +835,25 @@ static void get_id_serial_short(void *obj, void *data, QGuestAllocator *t_alloc) qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc); }
+static void get_id_serial_short_trunc(void *obj, void *data, + QGuestAllocator *t_alloc) +{ + QVirtioBlk *blk_if = obj; + QVirtioDevice *dev = blk_if->vdev; + QVirtQueue *vq; + uint8_t status; + /* Everything after the 20 character maximum is untouched, no '\0' */ + const char expected[25] = "01234567890123456789\xa5\xa5\xa5\xa5\xa5";
get_id_request() also uses the 0xa5 byte pattern. How about using a different pattern like 0x5a here to distinguish the two?
+ char buf[25]; + + vq = get_id_setup(dev, t_alloc); + status = get_id_request(dev, vq, buf, 25, t_alloc); + g_assert_cmpint(status, ==, 0); + g_assert_cmpmem(buf, 25, expected, sizeof(expected)); + + qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc); +} + static void *virtio_blk_test_setup(GString *cmd_line, void *arg) { char *tmp_path = drive_create(); @@ -871,6 +890,10 @@ static void register_virtio_blk_test(void) opts.edge.extra_device_opts = "serial=abc"; qos_add_test("get_id/serial_short", "virtio-blk", get_id_serial_short, &opts); + + opts.edge.extra_device_opts = "serial=012345678901234567890123456789"; + qos_add_test("get_id/serial_short_trunc", "virtio-blk", + get_id_serial_short_trunc, &opts); }
libqos_init(register_virtio_blk_test); -- 2.56.0
Am 29.10.2026 um 13:42 hat Stefan Hajnoczi geschrieben:
On Thu, Oct 29, 2026 at 12:24:00AM +0100, Kevin Wolf wrote:
Only 20 bytes are written to the passed buffer and the returned string doesn't have a NUL terminator.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qtest/virtio-blk-test.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c index 14fadcb4009..99d785d9e1a 100644 --- a/tests/qtest/virtio-blk-test.c +++ b/tests/qtest/virtio-blk-test.c @@ -835,6 +835,25 @@ static void get_id_serial_short(void *obj, void *data, QGuestAllocator *t_alloc) qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc); }
+static void get_id_serial_short_trunc(void *obj, void *data, + QGuestAllocator *t_alloc) +{ + QVirtioBlk *blk_if = obj; + QVirtioDevice *dev = blk_if->vdev; + QVirtQueue *vq; + uint8_t status; + /* Everything after the 20 character maximum is untouched, no '\0' */ + const char expected[25] = "01234567890123456789\xa5\xa5\xa5\xa5\xa5";
get_id_request() also uses the 0xa5 byte pattern. How about using a different pattern like 0x5a here to distinguish the two?
This string is used to verify that the 0xa5 pattern set by get_id_request() is still unchanged after the request, so the whole point is that it is the same. Kevin
+ char buf[25]; + + vq = get_id_setup(dev, t_alloc); + status = get_id_request(dev, vq, buf, 25, t_alloc); + g_assert_cmpint(status, ==, 0); + g_assert_cmpmem(buf, 25, expected, sizeof(expected)); + + qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc); +} + static void *virtio_blk_test_setup(GString *cmd_line, void *arg) { char *tmp_path = drive_create(); @@ -871,6 +890,10 @@ static void register_virtio_blk_test(void) opts.edge.extra_device_opts = "serial=abc"; qos_add_test("get_id/serial_short", "virtio-blk", get_id_serial_short, &opts); + + opts.edge.extra_device_opts = "serial=012345678901234567890123456789"; + qos_add_test("get_id/serial_short_trunc", "virtio-blk", + get_id_serial_short_trunc, &opts); }
libqos_init(register_virtio_blk_test); -- 2.56.0
On Thu, Oct 29, 2026 at 02:19:54PM +0100, Kevin Wolf wrote:
Am 29.10.2026 um 13:42 hat Stefan Hajnoczi geschrieben:
On Thu, Oct 29, 2026 at 12:24:00AM +0100, Kevin Wolf wrote:
Only 20 bytes are written to the passed buffer and the returned string doesn't have a NUL terminator.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qtest/virtio-blk-test.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c index 14fadcb4009..99d785d9e1a 100644 --- a/tests/qtest/virtio-blk-test.c +++ b/tests/qtest/virtio-blk-test.c @@ -835,6 +835,25 @@ static void get_id_serial_short(void *obj, void *data, QGuestAllocator *t_alloc) qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc); }
+static void get_id_serial_short_trunc(void *obj, void *data, + QGuestAllocator *t_alloc) +{ + QVirtioBlk *blk_if = obj; + QVirtioDevice *dev = blk_if->vdev; + QVirtQueue *vq; + uint8_t status; + /* Everything after the 20 character maximum is untouched, no '\0' */ + const char expected[25] = "01234567890123456789\xa5\xa5\xa5\xa5\xa5";
get_id_request() also uses the 0xa5 byte pattern. How about using a different pattern like 0x5a here to distinguish the two?
This string is used to verify that the 0xa5 pattern set by get_id_request() is still unchanged after the request, so the whole point is that it is the same.
Ah, I see now. How about passing in a pattern_byte argument into get_id_request() so that the connection is explicit? Stefan
On Thu, Oct 29, 2026 at 12:23:55AM +0100, Kevin Wolf wrote:
QEMU has allowed the user to specify serials of arbitrary length for virtio-blk devices, even though the virtio-blk protocol can only communicate 20 characters of the serial to the guest. This means that a longer serial number on the command line is silently truncated to 20 characters from the guest perspective.
As a first step to fix this, deprecate the silent truncation and turn it into a warning; we can later make this a hard error. While touching GET_ID, let's make it more spec compliant and clean up the internal state.
Another part is planned that will actually enable users to use longer serials and additional device identifiers. This will need spec changes and would take more time than we have until the QEMU 11.2 freeze anyway, so I'll address this later. For now, the important thing is to get the deprecation period started.
Kevin Wolf (5): virtio-blk: Implement full NUL padding for GET_ID virtio-blk-test: Add a basic GET_ID test case virtio-blk: Deprecate silent truncation of serial virtio-blk: Represent truncated serial in qdev property virtio-blk-test: Test truncated serial
docs/about/deprecated.rst | 13 +++++ hw/block/virtio-blk.c | 24 +++++++-- tests/qtest/virtio-blk-test.c | 92 +++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 4 deletions(-)
Aside from the comments I've sent: Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
participants (2)
-
Kevin Wolf -
Stefan Hajnoczi