[libvirt PATCH v2] qemu: drop support for monitor connections on PTYs

Daniel P. Berrangé posted 1 patch 2 weeks ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200212114108.3041280-1-berrange@redhat.com
src/qemu/qemu_monitor.c | 60 +++++++----------------------------------
1 file changed, 9 insertions(+), 51 deletions(-)

[libvirt PATCH v2] qemu: drop support for monitor connections on PTYs

Posted by Daniel P. Berrangé 2 weeks ago
Libvirt switched to using a UNIX socket for monitors in
2009 for version 0.7.0. It seems unlikely that there is
a running QEMU process that hasn't been restarted for
11 years while also taking a libvirt upgrade. Therefore
we can drop support for opening a PTY for the QEMU
monitor.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
In v2:

 - Now with all changes actually committed

 src/qemu/qemu_monitor.c | 60 +++++++----------------------------------
 1 file changed, 9 insertions(+), 51 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 802ad20aa1..008d4a0e75 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -77,7 +77,6 @@ struct _qemuMonitor {
      * = 0: not registered
      * < 0: an error occurred during the registration of @fd */
     int watch;
-    int hasSendFD;
 
     virDomainObjPtr vm;
 
@@ -303,21 +302,6 @@ qemuMonitorOpenUnix(const char *monitor,
 }
 
 
-static int
-qemuMonitorOpenPty(const char *monitor)
-{
-    int monfd;
-
-    if ((monfd = open(monitor, O_RDWR)) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to open monitor path %s"), monitor);
-        return -1;
-    }
-
-    return monfd;
-}
-
-
 /* This method processes data that has been received
  * from the monitor. Looking for async events and
  * replies/errors.
@@ -434,12 +418,6 @@ qemuMonitorIOWrite(qemuMonitorPtr mon)
     if (!mon->msg || mon->msg->txOffset == mon->msg->txLength)
         return 0;
 
-    if (mon->msg->txFD != -1 && !mon->hasSendFD) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Monitor does not support sending of file descriptors"));
-        return -1;
-    }
-
     buf = mon->msg->txBuffer + mon->msg->txOffset;
     len = mon->msg->txLength - mon->msg->txOffset;
     if (mon->msg->txFD == -1)
@@ -707,7 +685,6 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque)
 static qemuMonitorPtr
 qemuMonitorOpenInternal(virDomainObjPtr vm,
                         int fd,
-                        bool hasSendFD,
                         qemuMonitorCallbacksPtr cb,
                         void *opaque)
 {
@@ -736,7 +713,6 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
         goto cleanup;
     }
     mon->fd = fd;
-    mon->hasSendFD = hasSendFD;
     mon->vm = virObjectRef(vm);
     mon->waitGreeting = true;
     mon->cb = cb;
@@ -810,7 +786,6 @@ qemuMonitorOpen(virDomainObjPtr vm,
                 void *opaque)
 {
     int fd = -1;
-    bool hasSendFD = false;
     qemuMonitorPtr ret = NULL;
 
     timeout += QEMU_DEFAULT_MONITOR_WAIT;
@@ -819,28 +794,18 @@ qemuMonitorOpen(virDomainObjPtr vm,
      * deleted until the monitor gets its own reference. */
     virObjectRef(vm);
 
-    switch (config->type) {
-    case VIR_DOMAIN_CHR_TYPE_UNIX:
-        hasSendFD = true;
-        virObjectUnlock(vm);
-        fd = qemuMonitorOpenUnix(config->data.nix.path,
-                                 vm->pid, retry, timeout);
-        virObjectLock(vm);
-        break;
-
-    case VIR_DOMAIN_CHR_TYPE_PTY:
-        virObjectUnlock(vm);
-        fd = qemuMonitorOpenPty(config->data.file.path);
-        virObjectLock(vm);
-        break;
-
-    default:
+    if (config->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unable to handle monitor type: %s"),
                        virDomainChrTypeToString(config->type));
-        break;
+        goto cleanup;
     }
 
+    virObjectUnlock(vm);
+    fd = qemuMonitorOpenUnix(config->data.nix.path,
+                             vm->pid, retry, timeout);
+    virObjectLock(vm);
+
     if (fd < 0)
         goto cleanup;
 
@@ -850,7 +815,7 @@ qemuMonitorOpen(virDomainObjPtr vm,
         goto cleanup;
     }
 
-    ret = qemuMonitorOpenInternal(vm, fd, hasSendFD, cb, opaque);
+    ret = qemuMonitorOpenInternal(vm, fd, cb, opaque);
  cleanup:
     if (!ret)
         VIR_FORCE_CLOSE(fd);
@@ -865,7 +830,7 @@ qemuMonitorOpenFD(virDomainObjPtr vm,
                   qemuMonitorCallbacksPtr cb,
                   void *opaque)
 {
-    return qemuMonitorOpenInternal(vm, sockfd, true, cb, opaque);
+    return qemuMonitorOpenInternal(vm, sockfd, cb, opaque);
 }
 
 
@@ -2675,13 +2640,6 @@ qemuMonitorSendFileHandle(qemuMonitorPtr mon,
         return -1;
     }
 
-    if (!mon->hasSendFD) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-                       _("qemu is not using a unix socket monitor, "
-                         "cannot send fd %s"), fdname);
-        return -1;
-    }
-
     return qemuMonitorJSONSendFileHandle(mon, fdname, fd);
 }
 
-- 
2.24.1

Re: [libvirt PATCH v2] qemu: drop support for monitor connections on PTYs

Posted by Peter Krempa 2 weeks ago
On Wed, Feb 12, 2020 at 11:41:08 +0000, Daniel Berrange wrote:
> Libvirt switched to using a UNIX socket for monitors in
> 2009 for version 0.7.0. It seems unlikely that there is
> a running QEMU process that hasn't been restarted for
> 11 years while also taking a libvirt upgrade. Therefore
> we can drop support for opening a PTY for the QEMU
> monitor.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> In v2:
> 
>  - Now with all changes actually committed

ACK