On 12/09/2014 01:48 AM, Luyao Huang wrote: > Add a func just check the base target type which > qemu support. But i still doubt this will be useful > , we already have a check when we try to start the > vm. And this check only check the target type, and > the other things will be done in virDomainChrDefParseXML. > The commit message needs some massaging. Essentially, this patch fixes the "condition" where if a guest has been started and someone uses attach-device with (or without) the --config option, then these checks will avoid the "next" guest being modified, correct? This will also cause an error earlier that patch 1/2 as qemuDomainChrInsert is called in the path before qemuDomainAttachDeviceLive > Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx> > --- > src/qemu/qemu_hotplug.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index b9a0cee..fe91ec7 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -1384,10 +1384,74 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, > > } > > +static int > +qemuDomainChrCheckDefSupport(virDomainChrDefPtr chr) > +{ > + int ret = -1; > + > + switch (chr->deviceType) { > + case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: > + switch ((virDomainChrSerialTargetType) chr->targetType) { > + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: > + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: > + ret = 0; > + break; > + > + default: Typically in switches listing other options rather than default: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unsupported serial target type %s for qemu"), > + NULLSTR(virDomainChrSerialTargetTypeToString(chr->targetType))); > + break; > + } > + break; > + > + case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: > + ret = 0; > + break; > + > + case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: > + switch ((virDomainChrChannelTargetType) chr->targetType) { > + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: > + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: > + ret = 0; > + break; > + > + default: Same, but: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST: > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unsupported channel target type %s for qemu"), > + NULLSTR(virDomainChrChannelTargetTypeToString(chr->targetType))); > + break; > + } > + break; > + > + case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: > + switch ((virDomainChrConsoleTargetType) chr->targetType) { > + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: > + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: > + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: > + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: > + ret = 0; > + break; > + > + default: Same, but: 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: I think perhaps this one is better than 1/2, but will see if my responses result in other opinions... John > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unsupported console target type %s for qemu"), > + NULLSTR(virDomainChrConsoleTargetTypeToString(chr->targetType))); > + break; > + } > + break; > + } > + > + return ret; > +} > + > int > qemuDomainChrInsert(virDomainDefPtr vmdef, > virDomainChrDefPtr chr) > { > + if (qemuDomainChrCheckDefSupport(chr) < 0) > + return -1; > + > if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && > chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) { > virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list