Re: [PATCH V2] Fix bug of attaching redirdev device

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 22.02.2016 17:44, Osier Yang wrote:
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1298070
> 
> The corresponding chardev must be attached first, otherwise the
> the qemu command line won't be complete (missing the host part),
> ---
>  src/qemu/qemu_hotplug.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index ee305e7..40cead7 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1381,6 +1381,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
>      int ret;
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virDomainDefPtr def = vm->def;
> +    char *charAlias = NULL;
>      char *devstr = NULL;
>  
>      if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> @@ -1391,6 +1392,10 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
>  
>      if (qemuAssignDeviceRedirdevAlias(vm->def, redirdev, -1) < 0)
>          goto error;
> +
> +    if (virAsprintf(&charAlias, "char%s", redirdev->info.alias) < 0)
> +        goto error;
> +
>      if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps)))
>          goto error;
>  
> @@ -1398,6 +1403,14 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
>          goto error;
>  
>      qemuDomainObjEnterMonitor(driver, vm);
> +    if (qemuMonitorAttachCharDev(priv->mon,
> +                                 charAlias,
> +                                 &(redirdev->source.chr)) < 0) {
> +        ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +        goto error;
> +    }
> +    VIR_FREE(charAlias);
> +
>      ret = qemuMonitorAddDevice(priv->mon, devstr);
>  
>      if (qemuDomainObjExitMonitor(driver, vm) < 0)
> @@ -1414,9 +1427,9 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
>      return 0;
>  
>   error:
> +    VIR_FREE(charAlias);
>      VIR_FREE(devstr);
>      return -1;
> -
>  }
>  
>  static int
> 


ACK with this squashed in:


diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 40cead7..dc76268 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1378,7 +1378,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
                                    virDomainObjPtr vm,
                                    virDomainRedirdevDefPtr redirdev)
 {
-    int ret;
+    int ret = -1;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virDomainDefPtr def = vm->def;
     char *charAlias = NULL;
@@ -1387,49 +1387,47 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
     if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("redirected devices are not supported by this QEMU"));
-        goto error;
+        return ret;
     }
 
     if (qemuAssignDeviceRedirdevAlias(vm->def, redirdev, -1) < 0)
-        goto error;
+        goto cleanup;
 
     if (virAsprintf(&charAlias, "char%s", redirdev->info.alias) < 0)
-        goto error;
+        goto cleanup;
 
     if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps)))
-        goto error;
+        goto cleanup;
 
     if (VIR_REALLOC_N(vm->def->redirdevs, vm->def->nredirdevs+1) < 0)
-        goto error;
+        goto cleanup;
 
     qemuDomainObjEnterMonitor(driver, vm);
     if (qemuMonitorAttachCharDev(priv->mon,
                                  charAlias,
                                  &(redirdev->source.chr)) < 0) {
         ignore_value(qemuDomainObjExitMonitor(driver, vm));
-        goto error;
+        goto audit;
     }
-    VIR_FREE(charAlias);
 
-    ret = qemuMonitorAddDevice(priv->mon, devstr);
+    if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
+        /* detach associated chardev on error */
+        qemuMonitorDetachCharDev(priv->mon, charAlias);
+        ignore_value(qemuDomainObjExitMonitor(driver, vm));
+        goto audit;
+    }
 
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
-        goto error;
-
-    virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0);
-    if (ret < 0)
-        goto error;
+        goto audit;
 
     vm->def->redirdevs[vm->def->nredirdevs++] = redirdev;
-
-    VIR_FREE(devstr);
-
-    return 0;
-
- error:
+    ret = 0;
+ audit:
+    virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0);
+ cleanup:
     VIR_FREE(charAlias);
     VIR_FREE(devstr);
-    return -1;
+    return ret;
 }
 
 static int



Michal

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