On Thu, Jan 13, 2022 at 03:11:51PM +0530, Ani Sinha wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d822533ccb..4130df0ed9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10718,6 +10718,8 @@ qemuBuildSerialChrDeviceProps(const virDomainDef *def, g_autoptr(virJSONValue) props = NULL; g_autofree char *chardev = g_strdup_printf("char%s", serial->info.alias); virQEMUCapsFlags caps; + const char *typestr; + int ret;
type should match the return type of this function.
It does match return type of virDomainChrSerialTargetModelTypeToString(): 47 #define VIR_ENUM_DECL(name) \ 48 const char *name ## TypeToString(int type); \
ret should be defined as virJSONValue.
"int" is correct: 358 int 359 virJSONValueObjectAdd(virJSONValue **objptr, ...)
I preferred your previous style to this one.
Below is cleaner IMO: we don't repeat code, and the flow is much clearer.
switch ((virDomainChrSerialTargetModel) serial->targetModel) { case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL: @@ -10750,11 +10752,24 @@ qemuBuildSerialChrDeviceProps(const virDomainDef *def, return NULL; }
- if (virJSONValueObjectAdd(&props, - "s:driver", virDomainChrSerialTargetModelTypeToString(serial->targetModel), - "s:chardev", chardev, - "s:id", serial->info.alias, - NULL) < 0) + typestr = virDomainChrSerialTargetModelTypeToString(serial->targetModel); + + if (serial->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) { + ret = virJSONValueObjectAdd(&props, + "s:driver", typestr, + "s:chardev", chardev, + "s:id", serial->info.alias, + "k:index", serial->target.port, + NULL); + } else { + ret = virJSONValueObjectAdd(&props, + "s:driver", typestr, + "s:chardev", chardev, + "s:id", serial->info.alias, + NULL); + } + + if (ret < 0) return NULL;
regards john