On 05/30/2018 09:42 AM, Erik Skultety wrote:
It should be the command line helper who takes care of the iteration rather than the caller.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_command.c | 60 +++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0b5ec4f2ba..1667b09a8b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5323,10 +5323,10 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, } break; case VIR_MDEV_MODEL_TYPE_LAST: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected vfio type '%d'"), subsys->u.mdev.model); + default: + virReportEnumRangeError(virMediatedDeviceModelType, + subsys->u.mdev.model); return -1; - break;
Separate and unrelated hunk... Reviewed-by: John Ferlan <jferlan@redhat.com> for a separated patch...
}
virCommandAddArg(cmd, "-device"); @@ -8042,26 +8042,41 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, static int qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, - virDomainGraphicsDefPtr graphics) + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) { - switch (graphics->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - return qemuBuildGraphicsSDLCommandLine(cfg, cmd, qemuCaps, graphics); + size_t i;
- case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, graphics); + for (i = 0; i < def->ngraphics; i++) { + virDomainGraphicsDefPtr graphics = def->graphics[i];
- case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - return qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, graphics); + switch (graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + if (qemuBuildGraphicsSDLCommandLine(cfg, cmd, + qemuCaps, graphics) < 0) + return -1;
- case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - case VIR_DOMAIN_GRAPHICS_TYPE_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported graphics type '%s'"), - virDomainGraphicsTypeToString(graphics->type)); - return -1; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + if (qemuBuildGraphicsVNCCommandLine(cfg, cmd, + qemuCaps, graphics) < 0) + return -1; + + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + if (qemuBuildGraphicsSPICECommandLine(cfg, cmd, + qemuCaps, graphics) < 0) + return -1; + + break; + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + break;
Slight change from previous, but probably still worthy of the: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported graphics type '%s'"), virDomainGraphicsTypeToString(graphics->type)); return -1; since both are VirtualBox based and not QEMU. With that change, Reviewed-by: John Ferlan <jferlan@redhat.com> For this hunk. John
+ case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + default: + virReportEnumRangeError(virDomainGraphicsType, graphics->type); + return -1; + } }
return 0; @@ -10131,11 +10146,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0) goto error;
- for (i = 0; i < def->ngraphics; ++i) { - if (qemuBuildGraphicsCommandLine(cfg, cmd, qemuCaps, - def->graphics[i]) < 0) - goto error; - } + if (qemuBuildGraphicsCommandLine(cfg, cmd, def, qemuCaps) < 0) + goto error;
if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) goto error;