[PATCH 0/3] nodedev: Storage device size fixes
Peter Krempa (3): virNodeDeviceCapStorageDefFormat: Don't check argument for virBufferEscapeString virNodeDeviceCapStorageDefFormat: Extract formatting of block size data virNodeDeviceCapStorageDefFormatBlocksize: Report sector size and count together src/conf/node_device_conf.c | 65 +++++++++++++++---------------------- 1 file changed, 27 insertions(+), 38 deletions(-) -- 2.43.0
virBufferEscapeString is specifically designed for formatting XMLs and thus skips the whole formatting if the singular string argument is NULL. Remove redundant conditions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f722ab37c6..64bece59aa 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -540,23 +540,13 @@ static void virNodeDeviceCapStorageDefFormat(virBuffer *buf, const virNodeDevCapData *data) { - virBufferEscapeString(buf, "<block>%s</block>\n", - data->storage.block); - if (data->storage.bus) - virBufferEscapeString(buf, "<bus>%s</bus>\n", - data->storage.bus); - if (data->storage.drive_type) - virBufferEscapeString(buf, "<drive_type>%s</drive_type>\n", - data->storage.drive_type); - if (data->storage.model) - virBufferEscapeString(buf, "<model>%s</model>\n", - data->storage.model); - if (data->storage.vendor) - virBufferEscapeString(buf, "<vendor>%s</vendor>\n", - data->storage.vendor); - if (data->storage.serial) - virBufferEscapeString(buf, "<serial>%s</serial>\n", - data->storage.serial); + virBufferEscapeString(buf, "<block>%s</block>\n", data->storage.block); + virBufferEscapeString(buf, "<bus>%s</bus>\n", data->storage.bus); + virBufferEscapeString(buf, "<drive_type>%s</drive_type>\n", data->storage.drive_type); + virBufferEscapeString(buf, "<model>%s</model>\n", data->storage.model); + virBufferEscapeString(buf, "<vendor>%s</vendor>\n", data->storage.vendor); + virBufferEscapeString(buf, "<serial>%s</serial>\n", data->storage.serial); + if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE) { int avl = data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE; @@ -566,10 +556,7 @@ virNodeDeviceCapStorageDefFormat(virBuffer *buf, "</media_available>\n", avl ? 1 : 0); virBufferAsprintf(buf, "<media_size>%llu</media_size>\n", data->storage.removable_media_size); - if (data->storage.media_label) - virBufferEscapeString(buf, - "<media_label>%s</media_label>\n", - data->storage.media_label); + virBufferEscapeString(buf, "<media_label>%s</media_label>\n", data->storage.media_label); if (data->storage.logical_block_size > 0) virBufferAsprintf(buf, "<logical_block_size>%llu" "</logical_block_size>\n", -- 2.43.0
Unfortunately the XML is designed in a weird way, where based on whether media in the device is removable the sizing is either part of a subelement or placed directly on top level. The logic itself is identical so it can be extracted into a function to simplify the formatter. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 64bece59aa..3af5c3b7ed 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -536,6 +536,20 @@ virNodeDeviceCapSCSIDefFormat(virBuffer *buf, } +static void +virNodeDeviceCapStorageDefFormatBlocksize(virBuffer *buf, + const virNodeDevCapData *data) +{ + if (data->storage.logical_block_size > 0) + virBufferAsprintf(buf, "<logical_block_size>%llu</logical_block_size>\n", + data->storage.logical_block_size); + + if (data->storage.num_blocks > 0) + virBufferAsprintf(buf, "<num_blocks>%llu</num_blocks>\n", + data->storage.num_blocks); +} + + static void virNodeDeviceCapStorageDefFormat(virBuffer *buf, const virNodeDevCapData *data) @@ -557,27 +571,14 @@ virNodeDeviceCapStorageDefFormat(virBuffer *buf, virBufferAsprintf(buf, "<media_size>%llu</media_size>\n", data->storage.removable_media_size); virBufferEscapeString(buf, "<media_label>%s</media_label>\n", data->storage.media_label); - if (data->storage.logical_block_size > 0) - virBufferAsprintf(buf, "<logical_block_size>%llu" - "</logical_block_size>\n", - data->storage.logical_block_size); - if (data->storage.num_blocks > 0) - virBufferAsprintf(buf, - "<num_blocks>%llu</num_blocks>\n", - data->storage.num_blocks); + virNodeDeviceCapStorageDefFormatBlocksize(buf, data); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</capability>\n"); } else { - virBufferAsprintf(buf, "<size>%llu</size>\n", - data->storage.size); - if (data->storage.logical_block_size > 0) - virBufferAsprintf(buf, "<logical_block_size>%llu" - "</logical_block_size>\n", - data->storage.logical_block_size); - if (data->storage.num_blocks > 0) - virBufferAsprintf(buf, "<num_blocks>%llu</num_blocks>\n", - data->storage.num_blocks); + virBufferAsprintf(buf, "<size>%llu</size>\n", data->storage.size); + virNodeDeviceCapStorageDefFormatBlocksize(buf, data); } + if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE) virBufferAddLit(buf, "<capability type='hotpluggable'/>\n"); } -- 2.43.0
Report both block count and size together when either one of them is present equivalently to what the schema type 'blockData' in 'schemas/nodedev.rng' defines. Resolves: https://issues.redhat.com/browse/RHEL-18165 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 3af5c3b7ed..4826be6f42 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -540,13 +540,14 @@ static void virNodeDeviceCapStorageDefFormatBlocksize(virBuffer *buf, const virNodeDevCapData *data) { - if (data->storage.logical_block_size > 0) - virBufferAsprintf(buf, "<logical_block_size>%llu</logical_block_size>\n", - data->storage.logical_block_size); + if (data->storage.logical_block_size == 0 && + data->storage.num_blocks == 0) + return; - if (data->storage.num_blocks > 0) - virBufferAsprintf(buf, "<num_blocks>%llu</num_blocks>\n", - data->storage.num_blocks); + virBufferAsprintf(buf, "<logical_block_size>%llu</logical_block_size>\n", + data->storage.logical_block_size); + virBufferAsprintf(buf, "<num_blocks>%llu</num_blocks>\n", + data->storage.num_blocks); } -- 2.43.0
On a Thursday in 2023, Peter Krempa wrote:
Peter Krempa (3): virNodeDeviceCapStorageDefFormat: Don't check argument for virBufferEscapeString virNodeDeviceCapStorageDefFormat: Extract formatting of block size data virNodeDeviceCapStorageDefFormatBlocksize: Report sector size and count together
src/conf/node_device_conf.c | 65 +++++++++++++++---------------------- 1 file changed, 27 insertions(+), 38 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa