7:27 a.m.
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
7:27 a.m.
New subject: [PATCH 1/3] virNodeDeviceCapStorageDefFormat: Don't check argument for virBufferEscapeString
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(a)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
7:27 a.m.
New subject: [PATCH 2/3] virNodeDeviceCapStorageDefFormat: Extract formatting of block size data
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(a)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
7:27 a.m.
New subject: [PATCH 3/3] virNodeDeviceCapStorageDefFormatBlocksize: Report sector size and count together
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(a)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
8:19 a.m.
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(a)redhat.com>
Jano
384
days inactive
384
days old
4 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa