[libvirt] [PATCH 0/7] conf: Rework virDomainDeviceDefPostParseCommon()
Split actual functionality into a series of small helpers and turn it into a dispatcher. Andrea Bolognani (7): conf: Introduce virDomainChrDefPostParse() conf: Introduce virDomainRNGDefPostParse() conf: Introduce virDomainDiskDefPostParse() conf: Introduce virDomainVideoDefPostParse() conf: Introduce virDomainControllerDefPostParse() conf: Introduce virDomainNetDefPostParse() conf: Rework virDomainDeviceDefPostParseCommon() src/conf/domain_conf.c | 285 +++++++++++++++++++++++++++-------------- 1 file changed, 187 insertions(+), 98 deletions(-) -- 2.20.1
Minor tweaks to ensure compliance with our coding style. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 59 ++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5d49f4388c..74bb18d726 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4810,6 +4810,38 @@ virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio) } +static int +virDomainChrDefPostParse(virDomainChrDefPtr chr, + const virDomainDef *def) +{ + const virDomainChrDef **arrPtr; + size_t i, cnt; + + virDomainChrGetDomainPtrs(def, chr->deviceType, &arrPtr, &cnt); + + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) { + chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + } + + if (chr->target.port == -1 && + (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL || + chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL || + chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE)) { + int maxport = -1; + + for (i = 0; i < cnt; i++) { + if (arrPtr[i]->target.port > maxport) + maxport = arrPtr[i]->target.port; + } + + chr->target.port = maxport + 1; + } + + return 0; +} + + static int virDomainVsockDefPostParse(virDomainVsockDefPtr vsock) { @@ -4831,31 +4863,8 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, unsigned int parseFlags ATTRIBUTE_UNUSED, virDomainXMLOptionPtr xmlopt) { - if (dev->type == VIR_DOMAIN_DEVICE_CHR) { - virDomainChrDefPtr chr = dev->data.chr; - const virDomainChrDef **arrPtr; - size_t i, cnt; - - virDomainChrGetDomainPtrs(def, chr->deviceType, &arrPtr, &cnt); - - if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) - chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; - - if (chr->target.port == -1 && - (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL || - chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL || - chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE)) { - int maxport = -1; - - for (i = 0; i < cnt; i++) { - if (arrPtr[i]->target.port > maxport) - maxport = arrPtr[i]->target.port; - } - - chr->target.port = maxport + 1; - } - } + if (dev->type == VIR_DOMAIN_DEVICE_CHR) + return virDomainChrDefPostParse(dev->data.chr, def); /* set default path for virtio-rng "random" backend to /dev/random */ if (dev->type == VIR_DOMAIN_DEVICE_RNG && -- 2.20.1
On Fri, Feb 15, 2019 at 12:55:46PM +0100, Andrea Bolognani wrote:
Minor tweaks to ensure compliance with our coding style.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 59 ++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 25 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5d49f4388c..74bb18d726 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4810,6 +4810,38 @@ virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio) }
+static int +virDomainChrDefPostParse(virDomainChrDefPtr chr, + const virDomainDef *def) +{ + const virDomainChrDef **arrPtr;
Arrred-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 74bb18d726..99319578f7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4842,6 +4842,19 @@ virDomainChrDefPostParse(virDomainChrDefPtr chr, } +static int +virDomainRNGDefPostParse(virDomainRNGDefPtr rng) +{ + /* set default path for virtio-rng "random" backend to /dev/random */ + if (rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM && + !rng->source.file) { + if (VIR_STRDUP(rng->source.file, "/dev/random") < 0) + return -1; + } + + return 0; +} + static int virDomainVsockDefPostParse(virDomainVsockDefPtr vsock) { @@ -4866,13 +4879,8 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_CHR) return virDomainChrDefPostParse(dev->data.chr, def); - /* set default path for virtio-rng "random" backend to /dev/random */ - if (dev->type == VIR_DOMAIN_DEVICE_RNG && - dev->data.rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM && - !dev->data.rng->source.file) { - if (VIR_STRDUP(dev->data.rng->source.file, "/dev/random") < 0) - return -1; - } + if (dev->type == VIR_DOMAIN_DEVICE_RNG) + return virDomainRNGDefPostParse(dev->data.rng); /* verify disk source */ if (dev->type == VIR_DOMAIN_DEVICE_DISK) { -- 2.20.1
On Fri, Feb 15, 2019 at 12:55:47PM +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 74bb18d726..99319578f7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4842,6 +4842,19 @@ virDomainChrDefPostParse(virDomainChrDefPtr chr, }
+static int +virDomainRNGDefPostParse(virDomainRNGDefPtr rng) +{ + /* set default path for virtio-rng "random" backend to /dev/random */
"Reviewed"-by: Ján Tomko <jtomko@redhat.com> Jano
Minor tweaks to ensure compliance with our coding style. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 83 ++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 99319578f7..b794200e47 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4855,6 +4855,51 @@ virDomainRNGDefPostParse(virDomainRNGDefPtr rng) return 0; } + +static int +virDomainDiskDefPostParse(virDomainDiskDefPtr disk, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt) +{ + /* internal snapshots and config files are currently supported + * only with rbd: */ + if (virStorageSourceGetActualType(disk->src) != VIR_STORAGE_TYPE_NETWORK && + disk->src->protocol != VIR_STORAGE_NET_PROTOCOL_RBD) { + if (disk->src->snapshot) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("<snapshot> element is currently supported " + "only with 'rbd' disks")); + return -1; + } + + if (disk->src->configFile) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("<config> element is currently supported " + "only with 'rbd' disks")); + return -1; + } + } + + if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && + virDomainPostParseCheckISCSIPath(&disk->src->path) < 0) { + return -1; + } + + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + virDomainCheckVirtioOptions(disk->virtio) < 0) { + return -1; + } + + if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) { + return -1; + } + + return 0; +} + + static int virDomainVsockDefPostParse(virDomainVsockDefPtr vsock) { @@ -4882,42 +4927,8 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_RNG) return virDomainRNGDefPostParse(dev->data.rng); - /* verify disk source */ - if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - virDomainDiskDefPtr disk = dev->data.disk; - - /* internal snapshots and config files are currently supported - * only with rbd: */ - if (virStorageSourceGetActualType(disk->src) != VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol != VIR_STORAGE_NET_PROTOCOL_RBD) { - if (disk->src->snapshot) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("<snapshot> element is currently supported " - "only with 'rbd' disks")); - return -1; - } - - if (disk->src->configFile) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("<config> element is currently supported " - "only with 'rbd' disks")); - return -1; - } - } - - if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && - virDomainPostParseCheckISCSIPath(&disk->src->path) < 0) - return -1; - - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && - virDomainCheckVirtioOptions(disk->virtio) < 0) - return -1; - - if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) - return -1; - } + if (dev->type == VIR_DOMAIN_DEVICE_DISK) + return virDomainDiskDefPostParse(dev->data.disk, def, xmlopt); if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) { virDomainVideoDefPtr video = dev->data.video; -- 2.20.1
On Fri, Feb 15, 2019 at 12:55:48PM +0100, Andrea Bolognani wrote:
Minor tweaks to ensure compliance with our coding style.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 83 ++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 36 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 99319578f7..b794200e47 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4855,6 +4855,51 @@ virDomainRNGDefPostParse(virDomainRNGDefPtr rng) return 0; }
+
^ Whitespace Watch would worry.
+static int +virDomainDiskDefPostParse(virDomainDiskDefPtr disk, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt)
Unintended indentation ---^
+{
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Fri, 2019-02-15 at 14:25 +0100, Ján Tomko wrote:
On Fri, Feb 15, 2019 at 12:55:48PM +0100, Andrea Bolognani wrote:
@@ -4855,6 +4855,51 @@ virDomainRNGDefPostParse(virDomainRNGDefPtr rng) return 0; }
+
^ Whitespace Watch would worry.
That's because I added one empty line between functions instead of two in the previous commit. Good catch!
+static int +virDomainDiskDefPostParse(virDomainDiskDefPtr disk, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt)
Unintended indentation ---^
*unindented -- Andrea Bolognani / Red Hat / Virtualization
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b794200e47..4cfdfb230e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4900,6 +4900,23 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk, } +static int +virDomainVideoDefPostParse(virDomainVideoDefPtr video, + const virDomainDef *def) +{ + /* Fill out (V)RAM if the driver-specific callback did not do so */ + if (video->ram == 0 && video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) + video->ram = virDomainVideoDefaultRAM(def, video->type); + if (video->vram == 0) + video->vram = virDomainVideoDefaultRAM(def, video->type); + + video->ram = VIR_ROUND_UP_POWER_OF_TWO(video->ram); + video->vram = VIR_ROUND_UP_POWER_OF_TWO(video->vram); + + return 0; +} + + static int virDomainVsockDefPostParse(virDomainVsockDefPtr vsock) { @@ -4930,17 +4947,8 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_DISK) return virDomainDiskDefPostParse(dev->data.disk, def, xmlopt); - if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) { - virDomainVideoDefPtr video = dev->data.video; - /* Fill out (V)RAM if the driver-specific callback did not do so */ - if (video->ram == 0 && video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) - video->ram = virDomainVideoDefaultRAM(def, video->type); - if (video->vram == 0) - video->vram = virDomainVideoDefaultRAM(def, video->type); - - video->ram = VIR_ROUND_UP_POWER_OF_TWO(video->ram); - video->vram = VIR_ROUND_UP_POWER_OF_TWO(video->vram); - } + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) + return virDomainVideoDefPostParse(dev->data.video, def); if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && virDomainHostdevDefPostParse(dev->data.hostdev, def, xmlopt) < 0) -- 2.20.1
On Fri, Feb 15, 2019 at 12:55:49PM +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4cfdfb230e..872cb3352b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4917,6 +4917,22 @@ virDomainVideoDefPostParse(virDomainVideoDefPtr video, } +static int +virDomainControllerDefPostParse(virDomainControllerDefPtr cdev) +{ + if (cdev->iothread && + cdev->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) { + virReportError(VIR_ERR_XML_ERROR, + _("'iothread' attribute only supported for " + "controller model '%s'"), + virDomainControllerModelSCSITypeToString(VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)); + return -1; + } + + return 0; +} + + static int virDomainVsockDefPostParse(virDomainVsockDefPtr vsock) { @@ -4954,18 +4970,8 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, virDomainHostdevDefPostParse(dev->data.hostdev, def, xmlopt) < 0) return -1; - if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { - virDomainControllerDefPtr cdev = dev->data.controller; - - if (cdev->iothread && - cdev->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) { - virReportError(VIR_ERR_XML_ERROR, - _("'iothread' attribute only supported for " - "controller model '%s'"), - virDomainControllerModelSCSITypeToString(VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)); - return -1; - } - } + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) + return virDomainControllerDefPostParse(dev->data.controller); if (dev->type == VIR_DOMAIN_DEVICE_NET) { virDomainNetDefPtr net = dev->data.net; -- 2.20.1
On Fri, Feb 15, 2019 at 12:55:50PM +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Minor tweaks to ensure compliance with our coding style. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 872cb3352b..7f66fa27ff 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4933,6 +4933,18 @@ virDomainControllerDefPostParse(virDomainControllerDefPtr cdev) } +static int +virDomainNetDefPostParse(virDomainNetDefPtr net) +{ + if (!virDomainNetIsVirtioModel(net) && + virDomainCheckVirtioOptions(net->virtio) < 0) { + return -1; + } + + return 0; +} + + static int virDomainVsockDefPostParse(virDomainVsockDefPtr vsock) { @@ -4973,12 +4985,8 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) return virDomainControllerDefPostParse(dev->data.controller); - if (dev->type == VIR_DOMAIN_DEVICE_NET) { - virDomainNetDefPtr net = dev->data.net; - if (!virDomainNetIsVirtioModel(net) && - virDomainCheckVirtioOptions(net->virtio) < 0) - return -1; - } + if (dev->type == VIR_DOMAIN_DEVICE_NET) + return virDomainNetDefPostParse(dev->data.net); if (dev->type == VIR_DOMAIN_DEVICE_VSOCK && virDomainVsockDefPostParse(dev->data.vsock) < 0) -- 2.20.1
On Fri, Feb 15, 2019 at 12:55:51PM +0100, Andrea Bolognani wrote:
Minor tweaks to ensure compliance with our coding style.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Now that we've moved all the actual code into helper functions, we can turn it into a switch statement. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7f66fa27ff..ffdc027983 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4966,33 +4966,72 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, unsigned int parseFlags ATTRIBUTE_UNUSED, virDomainXMLOptionPtr xmlopt) { - if (dev->type == VIR_DOMAIN_DEVICE_CHR) - return virDomainChrDefPostParse(dev->data.chr, def); + int ret = -1; - if (dev->type == VIR_DOMAIN_DEVICE_RNG) - return virDomainRNGDefPostParse(dev->data.rng); + switch ((virDomainDeviceType)dev->type) { + case VIR_DOMAIN_DEVICE_CHR: + ret = virDomainChrDefPostParse(dev->data.chr, def); + break; - if (dev->type == VIR_DOMAIN_DEVICE_DISK) - return virDomainDiskDefPostParse(dev->data.disk, def, xmlopt); + case VIR_DOMAIN_DEVICE_RNG: + ret = virDomainRNGDefPostParse(dev->data.rng); + break; - if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) - return virDomainVideoDefPostParse(dev->data.video, def); + case VIR_DOMAIN_DEVICE_DISK: + ret = virDomainDiskDefPostParse(dev->data.disk, def, xmlopt); + break; - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - virDomainHostdevDefPostParse(dev->data.hostdev, def, xmlopt) < 0) - return -1; + case VIR_DOMAIN_DEVICE_VIDEO: + ret = virDomainVideoDefPostParse(dev->data.video, def); + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: + ret = virDomainHostdevDefPostParse(dev->data.hostdev, def, xmlopt); + break; - if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) - return virDomainControllerDefPostParse(dev->data.controller); + case VIR_DOMAIN_DEVICE_CONTROLLER: + ret = virDomainControllerDefPostParse(dev->data.controller); + break; - if (dev->type == VIR_DOMAIN_DEVICE_NET) - return virDomainNetDefPostParse(dev->data.net); + case VIR_DOMAIN_DEVICE_NET: + ret = virDomainNetDefPostParse(dev->data.net); + break; - if (dev->type == VIR_DOMAIN_DEVICE_VSOCK && - virDomainVsockDefPostParse(dev->data.vsock) < 0) - return -1; + case VIR_DOMAIN_DEVICE_VSOCK: + ret = virDomainVsockDefPostParse(dev->data.vsock); + break; - return 0; + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_IOMMU: + ret = 0; + break; + + case VIR_DOMAIN_DEVICE_NONE: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected VIR_DOMAIN_DEVICE_NONE")); + break; + + case VIR_DOMAIN_DEVICE_LAST: + default: + virReportEnumRangeError(virDomainDeviceType, dev->type); + break; + } + + return ret; } -- 2.20.1
On Fri, Feb 15, 2019 at 12:55:52PM +0100, Andrea Bolognani wrote:
Now that we've moved all the actual code into helper functions, we can turn it into a switch statement.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7f66fa27ff..ffdc027983 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4966,33 +4966,72 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, unsigned int parseFlags ATTRIBUTE_UNUSED, virDomainXMLOptionPtr xmlopt) { - if (dev->type == VIR_DOMAIN_DEVICE_CHR) - return virDomainChrDefPostParse(dev->data.chr, def); + int ret = -1;
With the preparatory patches adding an exit point in every if, I expected the 'return's to stay.
- if (dev->type == VIR_DOMAIN_DEVICE_RNG) - return virDomainRNGDefPostParse(dev->data.rng); + switch ((virDomainDeviceType)dev->type) {
[...]
+ case VIR_DOMAIN_DEVICE_NONE: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected VIR_DOMAIN_DEVICE_NONE")); + break; + + case VIR_DOMAIN_DEVICE_LAST: + default: + virReportEnumRangeError(virDomainDeviceType, dev->type); + break; + } + + return ret;
But I'm not sure how some tools would handle the (lack of) presence of this return. Either way: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
}
Andrea Bolognani <abologna@redhat.com> [2019-02-15, 12:55PM +0100]:
+ case VIR_DOMAIN_DEVICE_NONE: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected VIR_DOMAIN_DEVICE_NONE"));
Can we find a better error message here?
+ break; + + case VIR_DOMAIN_DEVICE_LAST: + default: + virReportEnumRangeError(virDomainDeviceType, dev->type); + break; + } + + return ret; }
-- 2.20.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- IBM Systems Linux on Z & Virtualization Development -------------------------------------------------- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -------------------------------------------------- Vorsitzende des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Mon, 2019-02-18 at 09:46 +0100, Bjoern Walk wrote:
Andrea Bolognani <abologna@redhat.com> [2019-02-15, 12:55PM +0100]:
+ case VIR_DOMAIN_DEVICE_NONE: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected VIR_DOMAIN_DEVICE_NONE"));
Can we find a better error message here?
Feel free to post a patch changing this (and the one in qemuDomainDeviceDefPostParse() that I've copied it from :), but it's one of those error messages that will only show up if we've made some pretty serious mistake somewhere leading up to the function being called, so I don't feel like it's particularly important for it to be user friendly. -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani -
Bjoern Walk -
Ján Tomko