Re: [libvirt] [PATCH] qemu: output error when try to hotplug unsupport console

On 11/26/2014 12:17 AM, Pavel Hrdina wrote: > On 11/17/2014 03:31 PM, Luyao Huang wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=1164627 >> >> When try to use attach-device to hotplug a qemu >> unsupport sonsole, command will return success, >> and add a console in vm's xml.But this doesn't work >> in qemu, qemu doesn't support these console.Add >> a error output when try to hotplug a unsupport console >> in qemuBuildConsoleChrDeviceStr. >> >> Signed-off-by: Luyao Huang <lhuang(a)redhat.com&gt; >> --- >> src/qemu/qemu_command.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index 1399ce4..2bf4a83 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -10069,13 +10069,17 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr, >> break; >> >> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: >> + break; >> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE: >> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN: >> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML: >> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC: >> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ: >> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST: >> - break; >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, >> + _("unsupported console target type %s"), >> + NULLSTR(virDomainChrConsoleTargetTypeToString(chr->targetType))); > > Wrong indentation Thanks for pointing out > >> + return ret; >> } >> >> ret = 0; >> > > Otherwise it seems good, bud what about the case that you will attach > unsupported device to offline domain? It is still possible but the > domain cannot be started afterwards. You should probably take care of > that too. > Eww, i thought that before, and i have a question about attach unsupported device to a offline domain. If we forbid attaching a unsupported device, is it means we should also forbid editing or defining a guest with a unsupported device?(because domain cannot start with these settings). I thinks it will be a big work. Actually, I just don't know when we try to set a invalid things to a offline domain, which way we should forbid and which way we can allow it.