qemu_fd: Don't rely on fdset id allocation by qemu

If we use our own fdset ID when hot-adding a fdset we can vastly
simplify our internals.

As a stop-gap when a fdset would be added behind libvirt's back we'll
validated that the fdset to be added is not yet used.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

src/qemu/qemu_fd.c

@@ -111,8 +111,12 @@ qemuFDPassNew(const char *prefix,
     fdpass->prefix = g_strdup(prefix);
     fdpass->useFDSet = true;
 
-    if (priv)
+    if (priv) {
         fdpass->fdSetID = qemuDomainFDSetIDNew(priv);
+        fdpass->path = g_strdup_printf("/dev/fdset/%u", fdpass->fdSetID);
+    } else {
+        fdpass->path = g_strdup_printf("/dev/fdset/monitor-fake");
+    }
 
     return fdpass;
 }
@@ -171,6 +175,11 @@ qemuFDPassAddFD(qemuFDPass *fdpass,
 
     newfd.opaque = g_strdup_printf("%s%s", fdpass->prefix, NULLSTR_EMPTY(suffix));
 
+    if (!fdpass->useFDSet) {
+        g_free(fdpass->path);
+        fdpass->path = g_strdup(newfd.opaque);
+    }
+
     VIR_APPEND_ELEMENT(fdpass->fds, fdpass->nfds, newfd);
 }
 
@@ -207,10 +216,10 @@ qemuFDPassTransferCommand(qemuFDPass *fdpass,
                                   fdpass->fds[i].opaque);
 
             virCommandAddArgList(cmd, "-add-fd", arg, NULL);
-
-            fdpass->path = g_strdup_printf("/dev/fdset/%u", fdpass->fdSetID);
         } else {
-            fdpass->path = g_strdup_printf("%u", fdpass->fds[i].fd);
+            /* for monitor use the older FD passing needs the FD number */
+            g_free(fdpass->path);
+            fdpass->path = g_strdup_printf("%d", fdpass->fds[i].fd);
         }
 
         fdpass->fds[i].fd = -1;
@@ -232,7 +241,6 @@ int
 qemuFDPassTransferMonitor(qemuFDPass *fdpass,
                           qemuMonitor *mon)
 {
-    int fdsetid = -1;
     size_t i;
 
     if (!fdpass)
@@ -240,6 +248,21 @@ qemuFDPassTransferMonitor(qemuFDPass *fdpass,
 
     if (qemuFDPassValidate(fdpass) < 0)
         return -1;
+    if (fdpass->useFDSet) {
+        g_autoptr(qemuMonitorFdsets) fdsets = NULL;
+
+        if (qemuMonitorQueryFdsets(mon, &fdsets) < 0)
+            return -1;
+
+        for (i = 0; i < fdsets->nfdsets; i++) {
+            if (fdsets->fdsets[i].id == fdpass->fdSetID) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("fdset '%u' is already in use by qemu"),
+                               fdpass->fdSetID);
+                return -1;
+            }
+        }
+    }
 
     for (i = 0; i < fdpass->nfds; i++) {
         if (fdpass->useFDSet) {
@@ -247,22 +270,15 @@ qemuFDPassTransferMonitor(qemuFDPass *fdpass,
 
             if (qemuMonitorAddFileHandleToSet(mon,
                                               fdpass->fds[i].fd,
-                                              fdsetid,
+                                              fdpass->fdSetID,
                                               fdpass->fds[i].opaque,
                                               &fdsetinfo) < 0)
                 return -1;
-
-            if (fdsetid == -1) {
-                fdpass->fdSetID = fdsetid = fdsetinfo.fdset;
-                fdpass->path = g_strdup_printf("/dev/fdset/%u", fdsetid);
-            }
         } else {
             if (qemuMonitorSendFileHandle(mon,
                                           fdpass->fds[i].opaque,
                                           fdpass->fds[i].fd) < 0)
                 return -1;
-
-            fdpass->path = g_strdup(fdpass->fds[i].opaque);
         }
 
         fdpass->passed = true;
@@ -272,33 +288,6 @@ qemuFDPassTransferMonitor(qemuFDPass *fdpass,
 }
 
 
-/**
- * qemuFDPassTransferMonitorFake:
- * @fdpass: The fd passing helper struct
- *
- * Simulate as if @fdpass was passed via monitor for callers which don't
- * actually wish to test that code path.
- */
-int
-qemuFDPassTransferMonitorFake(qemuFDPass *fdpass)
-{
-
-    if (!fdpass)
-        return 0;
-
-    if (qemuFDPassValidate(fdpass) < 0)
-        return -1;
-
-    if (fdpass->useFDSet) {
-        fdpass->path = g_strdup_printf("/dev/fdset/monitor-fake");
-    } else {
-        fdpass->path = g_strdup(fdpass->fds[0].opaque);
-    }
-
-    return 0;
-}
-
-
 /**
  * qemuFDPassTransferMonitorRollback:
  * @fdpass: The fd passing helper struct

src/qemu/qemu_fd.h

@@ -47,9 +47,6 @@ int
 qemuFDPassTransferMonitor(qemuFDPass *fdpass,
                           qemuMonitor *mon);
 
-int
-qemuFDPassTransferMonitorFake(qemuFDPass *fdpass);
-
 void
 qemuFDPassTransferMonitorRollback(qemuFDPass *fdpass,
                                   qemuMonitor *mon);

tests/qemuhotplugtest.c

@@ -834,6 +834,7 @@ mymain(void)
                    "device_del", QMP_DEVICE_DELETED("hostdev0") QMP_OK);
 
     DO_TEST_ATTACH("base-live", "interface-vdpa", false, true,
+                   "query-fdsets", "{\"return\":[{\"fdset-id\":99999}]}",
                    "add-fd", "{ \"return\": { \"fdset-id\": 1, \"fd\": 95 }}",
                    "netdev_add", QMP_OK, "device_add", QMP_OK);
     DO_TEST_DETACH("base-live", "interface-vdpa", false, false,

tests/qemumonitorjsontest.c

@@ -725,7 +725,6 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOption *xmlopt,
 
     {
         g_autoptr(virDomainChrSourceDef) chr = virDomainChrSourceDefNew(xmlopt);
-        qemuDomainChrSourcePrivate *charpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chr);
 
         chr->data.file.path = g_strdup("/test/path");
 
@@ -743,12 +742,6 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOption *xmlopt,
 
         chrdev.source = chr;
         ignore_value(testQemuPrepareHostBackendChardevOne(&dev, chr, NULL));
-        if (qemuFDPassTransferMonitorFake(charpriv->sourcefd) < 0)
-            ret = -1;
-
-        if (qemuFDPassTransferMonitorFake(charpriv->logfd) < 0)
-            ret = -1;
-
         CHECK("file", false,
               "{'id':'alias','backend':{'type':'file','data':{'out':'/dev/fdset/monitor-fake',"
                                                              "'append':true,"