On 9/4/25 14:10, Stefan Kober wrote:
Refactor BuildDiskJson to return a virJSONValue instead of adding the disk json to an json array. This makes the function reusable for hotplugging disks.
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_monitor.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index d369236183..f65cca648b 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -234,43 +234,41 @@ virCHMonitorBuildMemoryJson(virJSONValue *content, virDomainDef *vmdef) return 0; }
-static int -virCHMonitorBuildDiskJson(virJSONValue *disks, virDomainDiskDef *diskdef) +static virJSONValue* +virCHMonitorBuildDiskJson(virDomainDiskDef *diskdef) { g_autoptr(virJSONValue) disk = virJSONValueNewObject();
if (!diskdef->src) - return -1; + return NULL;
switch (diskdef->src->type) { case VIR_STORAGE_TYPE_FILE: if (!diskdef->src->path) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Missing disk file path in domain")); - return -1; + return NULL; } if (!diskdef->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing disk alias")); - return -1; + return NULL; } if (diskdef->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { virReportError(VIR_ERR_INVALID_ARG, _("Only virtio bus types are supported for '%1$s'"), diskdef->src->path); - return -1; + return NULL; } if (virJSONValueObjectAppendString(disk, "path", diskdef->src->path) < 0) - return -1; + return NULL; if (diskdef->src->readonly) { if (virJSONValueObjectAppendBoolean(disk, "readonly", true) < 0) - return -1; + return NULL; } if (virJSONValueObjectAppendString(disk, "id", diskdef->info.alias) < 0) { - return -1; + return NULL; } - if (virJSONValueArrayAppend(disks, &disk) < 0) - return -1;
break; case VIR_STORAGE_TYPE_NONE: @@ -284,23 +282,26 @@ virCHMonitorBuildDiskJson(virJSONValue *disks, virDomainDiskDef *diskdef) case VIR_STORAGE_TYPE_LAST: default: virReportEnumRangeError(virStorageType, diskdef->src->type); - return -1; + return NULL; }
- return 0; + return g_steal_pointer(&disk); }
static int virCHMonitorBuildDisksJson(virJSONValue *content, virDomainDef *vmdef) { g_autoptr(virJSONValue) disks = NULL; + g_autoptr(virJSONValue) disk = NULL;
This variable ...
size_t i;
if (vmdef->ndisks > 0) { disks = virJSONValueNewArray();
for (i = 0; i < vmdef->ndisks; i++) {
... is used exclusively in this loop. Declaring it here not only improves readability (I don't have to hold yet another variable inside my head when reading this function) but also avoids unexpected reuse of the variable. In fact, this whole function could be written a bit better. One level of nesting could be dropped, but that's outside of the scope of this particular patch.
- if (virCHMonitorBuildDiskJson(disks, vmdef->disks[i]) < 0) + if ((disk = virCHMonitorBuildDiskJson(vmdef->disks[i])) == NULL) + return -1; + if (virJSONValueArrayAppend(disks, &disk) < 0) return -1; } if (virJSONValueObjectAppend(content, "disks", &disks) < 0)
Michal