On 01/21/2015 03:00 AM, John Ferlan wrote:

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?

Right
This will also cause an error earlier that patch 1/2 as
qemuDomainChrInsert is called in the path before qemuDomainAttachDeviceLive



Yes and if this patch have been pushed then the patch 1/2 will be a patch for improving exist code.
Signed-off-by: Luyao Huang <lhu...@redhat.com>
---
  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...

Thanks for pointing out and i forgot cc Jan and Pavel when sent this patch, maybe they have some 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",


Luyao

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to