Add possibility to delay checks to the point when the FDs are to be passed to qemu. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 16 +++++++++++----- src/qemu/qemu_fd.c | 12 ++++++++---- src/qemu/qemu_fd.h | 4 ++-- tests/qemumonitorjsontest.c | 8 ++++++-- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3746f02ff0..30671df4f2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1494,7 +1494,8 @@ qemuBuildChardevCommand(virCommand *cmd, case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_UNIX: - qemuFDPassTransferCommand(chrSourcePriv->sourcefd, cmd); + if (qemuFDPassTransferCommand(chrSourcePriv->sourcefd, cmd) < 0) + return -1; break; case VIR_DOMAIN_CHR_TYPE_NULL: @@ -1517,7 +1518,8 @@ qemuBuildChardevCommand(virCommand *cmd, return -1; } - qemuFDPassTransferCommand(chrSourcePriv->logfd, cmd); + if (qemuFDPassTransferCommand(chrSourcePriv->logfd, cmd) < 0) + return -1; if (!(charstr = qemuBuildChardevStr(dev, charAlias))) return -1; @@ -8896,7 +8898,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, vhostfd[i] = -1; } - qemuFDPassTransferCommand(vdpa, cmd); + if (qemuFDPassTransferCommand(vdpa, cmd) < 0) + return -1; if (!(hostnetprops = qemuBuildHostNetProps(net, tapfdName, tapfdSize, @@ -9805,8 +9808,11 @@ qemuBuildTPMCommandLine(virCommand *cmd, return -1; } - qemuFDPassTransferCommand(passtpm, cmd); - qemuFDPassTransferCommand(passcancel, cmd); + if (qemuFDPassTransferCommand(passtpm, cmd) < 0) + return -1; + + if (qemuFDPassTransferCommand(passcancel, cmd) < 0) + return -1; if (!(tpmdevstr = qemuBuildTPMBackendStr(tpm, passtpm, passcancel))) return -1; diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c index 29370a4bb6..1af8932bdd 100644 --- a/src/qemu/qemu_fd.c +++ b/src/qemu/qemu_fd.c @@ -177,14 +177,14 @@ qemuFDPassAddFD(qemuFDPass *fdpass, * Pass the fds in @fdpass to a commandline object @cmd. @fdpass may be NULL * in which case this is a no-op. */ -void +int qemuFDPassTransferCommand(qemuFDPass *fdpass, virCommand *cmd) { size_t i; if (!fdpass) - return; + return 0; for (i = 0; i < fdpass->nfds; i++) { virCommandPassFD(cmd, fdpass->fds[i].fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); @@ -206,6 +206,8 @@ qemuFDPassTransferCommand(qemuFDPass *fdpass, fdpass->fds[i].fd = -1; } + + return 0; } @@ -265,18 +267,20 @@ qemuFDPassTransferMonitor(qemuFDPass *fdpass, * Simulate as if @fdpass was passed via monitor for callers which don't * actually wish to test that code path. */ -void +int qemuFDPassTransferMonitorFake(qemuFDPass *fdpass) { if (!fdpass) - return; + return 0; if (fdpass->useFDSet) { fdpass->path = g_strdup_printf("/dev/fdset/monitor-fake"); } else { fdpass->path = g_strdup(fdpass->fds[0].opaque); } + + return 0; } diff --git a/src/qemu/qemu_fd.h b/src/qemu/qemu_fd.h index 6d090392db..db16d77ecc 100644 --- a/src/qemu/qemu_fd.h +++ b/src/qemu/qemu_fd.h @@ -39,7 +39,7 @@ qemuFDPassAddFD(qemuFDPass *fdpass, int *fd, const char *suffix); -void +int qemuFDPassTransferCommand(qemuFDPass *fdpass, virCommand *cmd); @@ -47,7 +47,7 @@ int qemuFDPassTransferMonitor(qemuFDPass *fdpass, qemuMonitor *mon); -void +int qemuFDPassTransferMonitorFake(qemuFDPass *fdpass); void diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 278d7ba765..e15c8533f6 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -743,8 +743,12 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOption *xmlopt, chrdev.source = chr; ignore_value(testQemuPrepareHostBackendChardevOne(&dev, chr, NULL)); - qemuFDPassTransferMonitorFake(charpriv->sourcefd); - qemuFDPassTransferMonitorFake(charpriv->logfd); + 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," -- 2.35.1

Add validation to the transfer step to make the adding step more simple for easier cleanup paths. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_fd.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c index 1af8932bdd..284e77ba2b 100644 --- a/src/qemu/qemu_fd.c +++ b/src/qemu/qemu_fd.c @@ -65,6 +65,30 @@ qemuFDPassFree(qemuFDPass *fdpass) } +static int +qemuFDPassValidate(qemuFDPass *fdpass) +{ + size_t i; + + if (!fdpass->useFDSet && + fdpass->nfds > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("direct FD passing supports only 1 file descriptor")); + return -1; + } + + for (i = 0; i < fdpass->nfds; i++) { + if (fdpass->fds[i].fd < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid file descriptor")); + return -1; + } + } + + return 0; +} + + /** * qemuFDPassNew: * @prefix: prefix used for naming the passed FDs @@ -186,6 +210,9 @@ qemuFDPassTransferCommand(qemuFDPass *fdpass, if (!fdpass) return 0; + if (qemuFDPassValidate(fdpass) < 0) + return -1; + for (i = 0; i < fdpass->nfds; i++) { virCommandPassFD(cmd, fdpass->fds[i].fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); @@ -229,6 +256,9 @@ qemuFDPassTransferMonitor(qemuFDPass *fdpass, if (!fdpass) return 0; + if (qemuFDPassValidate(fdpass) < 0) + return -1; + for (i = 0; i < fdpass->nfds; i++) { if (fdpass->useFDSet) { qemuMonitorAddFdInfo fdsetinfo; @@ -274,6 +304,9 @@ 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 { -- 2.35.1

It's effectively replaced by checks in qemuFDPassTransfer. This will simplify cleanup paths on constructing the qemuFDPass object when FDs are being handled. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 13 ++++--------- src/qemu/qemu_fd.c | 20 +------------------- src/qemu/qemu_fd.h | 2 +- src/qemu/qemu_process.c | 9 +++------ tests/testutilsqemu.c | 6 ++---- 5 files changed, 11 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 30671df4f2..3032cbc623 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4927,10 +4927,7 @@ qemuBuildVideoCommandLine(virCommand *cmd, chrsrc->type = VIR_DOMAIN_CHR_TYPE_UNIX; chrsrcpriv->sourcefd = qemuFDPassNewDirect(video->info.alias, priv); - if (qemuFDPassAddFD(chrsrcpriv->sourcefd, - &videopriv->vhost_user_fd, - "-vhost-user") < 0) - return -1; + qemuFDPassAddFD(chrsrcpriv->sourcefd, &videopriv->vhost_user_fd, "-vhost-user"); if (qemuBuildChardevCommand(cmd, chrsrc, chrAlias, priv->qemuCaps) < 0) return -1; @@ -8763,8 +8760,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, vdpa = qemuFDPassNew(net->info.alias, priv); - if (qemuFDPassAddFD(vdpa, &vdpafd, "-vdpa") < 0) - return -1; + qemuFDPassAddFD(vdpa, &vdpafd, "-vdpa"); } break; @@ -9792,9 +9788,8 @@ qemuBuildTPMCommandLine(virCommand *cmd, passtpm = qemuFDPassNew(tpm->info.alias, priv); passcancel = qemuFDPassNew(tpm->info.alias, priv); - if (qemuFDPassAddFD(passtpm, &fdtpm, "-tpm") < 0 || - qemuFDPassAddFD(passcancel, &fdcancel, "-cancel") < 0) - return -1; + qemuFDPassAddFD(passtpm, &fdtpm, "-tpm"); + qemuFDPassAddFD(passcancel, &fdcancel, "-cancel"); } break; diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c index 284e77ba2b..84e9d2bfdf 100644 --- a/src/qemu/qemu_fd.c +++ b/src/qemu/qemu_fd.c @@ -159,37 +159,19 @@ qemuFDPassNewDirect(const char *prefix, * @suffix is used to build the name of the file descriptor by concatenating * it with @prefix passed to qemuFDPassNew. @suffix may be NULL, in which case * it's considered to be an empty string. - * - * Returns 0 on success, -1 on error (when attempting to pass multiple FDs) using - * the 'direct' method. */ -int +void qemuFDPassAddFD(qemuFDPass *fdpass, int *fd, const char *suffix) { struct qemuFDPassFD newfd = { .fd = *fd }; - if (newfd.fd < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("invalid file descriptor")); - return -1; - } - - if (!fdpass->useFDSet && - fdpass->nfds >= 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("direct FD passing supports only 1 file descriptor")); - return -1; - } - *fd = -1; newfd.opaque = g_strdup_printf("%s%s", fdpass->prefix, NULLSTR_EMPTY(suffix)); VIR_APPEND_ELEMENT(fdpass->fds, fdpass->nfds, newfd); - - return 0; } diff --git a/src/qemu/qemu_fd.h b/src/qemu/qemu_fd.h index db16d77ecc..ef35daba8c 100644 --- a/src/qemu/qemu_fd.h +++ b/src/qemu/qemu_fd.h @@ -34,7 +34,7 @@ qemuFDPass * qemuFDPassNewDirect(const char *prefix, void *dompriv); -int +void qemuFDPassAddFD(qemuFDPass *fdpass, int *fd, const char *suffix); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b0b00eb0a2..39ed10ee41 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6842,8 +6842,7 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev, charpriv->sourcefd = qemuFDPassNew(devalias, data->priv); - if (qemuFDPassAddFD(charpriv->sourcefd, &sourcefd, "-source") < 0) - return -1; + qemuFDPassAddFD(charpriv->sourcefd, &sourcefd, "-source"); } break; @@ -6862,8 +6861,7 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev, charpriv->sourcefd = qemuFDPassNewDirect(devalias, data->priv); - if (qemuFDPassAddFD(charpriv->sourcefd, &sourcefd, "-source") < 0) - return -1; + qemuFDPassAddFD(charpriv->sourcefd, &sourcefd, "-source"); } break; @@ -6890,8 +6888,7 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev, charpriv->logfd = qemuFDPassNew(devalias, data->priv); - if (qemuFDPassAddFD(charpriv->logfd, &logfd, "-log") < 0) - return -1; + qemuFDPassAddFD(charpriv->logfd, &logfd, "-log"); } return 0; diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index d57f982c37..44443d8585 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -1083,8 +1083,7 @@ testQemuPrepareHostBackendChardevOne(virDomainDeviceDef *dev, else charpriv->sourcefd = qemuFDPassNewDirect(devalias, priv); - if (qemuFDPassAddFD(charpriv->sourcefd, &fakesourcefd, "-source") < 0) - return -1; + qemuFDPassAddFD(charpriv->sourcefd, &fakesourcefd, "-source"); } if (chardev->logfile) { @@ -1095,8 +1094,7 @@ testQemuPrepareHostBackendChardevOne(virDomainDeviceDef *dev, charpriv->logfd = qemuFDPassNew(devalias, priv); - if (qemuFDPassAddFD(charpriv->logfd, &fd, "-log") < 0) - return -1; + qemuFDPassAddFD(charpriv->logfd, &fd, "-log"); } return 0; -- 2.35.1

We use the qemuFDPass infrastructure when building the command line, refactor the monitor too. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 274f47c25d..62cfc29a16 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1184,8 +1184,8 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, virErrorPtr originalError = NULL; g_autofree char *slirpfdName = NULL; int slirpfd = -1; - g_autofree char *vdpafdName = NULL; int vdpafd = -1; + g_autoptr(qemuFDPass) vdpa = NULL; char **tapfdName = NULL; int *tapfd = NULL; size_t tapfdSize = 0; @@ -1394,6 +1394,10 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0) goto cleanup; + + vdpa = qemuFDPassNew(net->info.alias, priv); + + qemuFDPassAddFD(vdpa, &vdpafd, "-vdpa"); break; case VIR_DOMAIN_NET_TYPE_SERVER: @@ -1455,24 +1459,15 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, qemuDomainObjEnterMonitor(driver, vm); - if (vdpafd > 0) { - /* vhost-vdpa only accepts a filename. We can pass an open fd by - * filename if we add the fd to an fdset and then pass a filename of - * /dev/fdset/$FDSETID. */ - qemuMonitorAddFdInfo fdinfo; - if (qemuMonitorAddFileHandleToSet(priv->mon, vdpafd, -1, - net->data.vdpa.devicepath, - &fdinfo) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; - } - vdpafdName = g_strdup_printf("/dev/fdset/%d", fdinfo.fdset); + if (qemuFDPassTransferMonitor(vdpa, priv->mon) < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; } if (!(netprops = qemuBuildHostNetProps(net, tapfdName, tapfdSize, vhostfdName, vhostfdSize, - slirpfdName, vdpafdName))) { + slirpfdName, qemuFDPassGetPath(vdpa)))) { qemuDomainObjExitMonitor(vm); goto cleanup; } -- 2.35.1

Libvirt doesn't use the returned value and in fact there's nothing we could even do with them. Avoid parsing and storing them. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor.h | 2 -- src/qemu/qemu_monitor_json.c | 12 ------------ 2 files changed, 14 deletions(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 5c2a749282..e6a50e73f7 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -943,7 +943,6 @@ int qemuMonitorGraphicsRelocate(qemuMonitor *mon, typedef struct _qemuMonitorAddFdInfo qemuMonitorAddFdInfo; struct _qemuMonitorAddFdInfo { - int fd; int fdset; }; int @@ -959,7 +958,6 @@ qemuMonitorRemoveFdset(qemuMonitor *mon, typedef struct _qemuMonitorFdsetFdInfo qemuMonitorFdsetFdInfo; struct _qemuMonitorFdsetFdInfo { - int fd; char *opaque; }; typedef struct _qemuMonitorFdsetInfo qemuMonitorFdsetInfo; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9e611e93e8..659d957b04 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3606,12 +3606,6 @@ qemuAddfdInfoParse(virJSONValue *msg, return -1; } - if (virJSONValueObjectGetNumberInt(returnObj, "fd", &fdinfo->fd) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing or invalid fd in add-fd response")); - return -1; - } - if (virJSONValueObjectGetNumberInt(returnObj, "fdset-id", &fdinfo->fdset) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing or invalid fdset-id in add-fd response")); @@ -3706,12 +3700,6 @@ qemuMonitorJSONQueryFdsetsParse(virJSONValue *msg, return -1; } - if (virJSONValueObjectGetNumberInt(fdentry, "fd", &fdinfo->fd) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-fdsets return data missing 'fd'")); - return -1; - } - /* opaque is optional and may be missing */ fdinfo->opaque = g_strdup(virJSONValueObjectGetString(fdentry, "opaque")); } -- 2.35.1

The code didn't check that the reply value is an array and that the 'fds' array is present. This could lead to a crash if qemu wouldn't return an array in those places. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor_json.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 659d957b04..6939eaea17 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3685,23 +3685,24 @@ qemuMonitorJSONQueryFdsetsParse(virJSONValue *msg, } - fdarray = virJSONValueObjectGetArray(entry, "fds"); - fdsetinfo->nfds = virJSONValueArraySize(fdarray); - if (fdsetinfo->nfds > 0) - fdsetinfo->fds = g_new0(qemuMonitorFdsetFdInfo, fdsetinfo->nfds); - - for (j = 0; j < fdsetinfo->nfds; j++) { - qemuMonitorFdsetFdInfo *fdinfo = &fdsetinfo->fds[j]; - virJSONValue *fdentry; - - if (!(fdentry = virJSONValueArrayGet(fdarray, j))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-fdsets return data missing fd array element")); - return -1; + if ((fdarray = virJSONValueObjectGetArray(entry, "fds"))) { + fdsetinfo->nfds = virJSONValueArraySize(fdarray); + if (fdsetinfo->nfds > 0) + fdsetinfo->fds = g_new0(qemuMonitorFdsetFdInfo, fdsetinfo->nfds); + + for (j = 0; j < fdsetinfo->nfds; j++) { + qemuMonitorFdsetFdInfo *fdinfo = &fdsetinfo->fds[j]; + virJSONValue *fdentry; + + if (!(fdentry = virJSONValueArrayGet(fdarray, j))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-fdsets return data missing fd array element")); + return -1; + } + + /* opaque is optional and may be missing */ + fdinfo->opaque = g_strdup(virJSONValueObjectGetString(fdentry, "opaque")); } - - /* opaque is optional and may be missing */ - fdinfo->opaque = g_strdup(virJSONValueObjectGetString(fdentry, "opaque")); } } @@ -3723,7 +3724,7 @@ int qemuMonitorJSONQueryFdsets(qemuMonitor *mon, if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) return -1; - if (qemuMonitorJSONCheckError(cmd, reply) < 0) + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0) return -1; if (qemuMonitorJSONQueryFdsetsParse(reply, fdsets) < 0) -- 2.35.1

While 'add-fd' qmp command gives the possibility to find an unused fdset ID when hot-adding fdsets, such usage is extremely inconvenient. This patch allows us to track the used fdset id so that we can avoid the need to check results and thus employ simpler code flow when hot-adding devices which use FD passing. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 5 +++ src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_process.c | 31 +++++++++++++++++++ .../qemustatusxml2xmldata/backup-pull-in.xml | 1 + .../blockjob-blockdev-in.xml | 1 + .../blockjob-mirror-in.xml | 1 + .../migration-in-params-in.xml | 1 + .../migration-out-nbd-bitmaps-in.xml | 1 + .../migration-out-nbd-out.xml | 1 + .../migration-out-nbd-tls-out.xml | 1 + .../migration-out-params-in.xml | 1 + tests/qemustatusxml2xmldata/modern-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-out.xml | 1 + .../qemustatusxml2xmldata/vcpus-multi-in.xml | 1 + 14 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7974cdb00b..a726e2624a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2372,6 +2372,8 @@ qemuDomainObjPrivateXMLFormat(virBuffer *buf, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) virBufferAsprintf(buf, "<nodename index='%llu'/>\n", priv->nodenameindex); + virBufferAsprintf(buf, "<fdset index='%u'/>\n", priv->fdsetindex); + if (priv->memPrealloc) virBufferAddLit(buf, "<memPrealloc/>\n"); @@ -3100,6 +3102,9 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, goto error; } + if (virXPathUInt("string(./fdset/@index)", ctxt, &priv->fdsetindex) == 0) + priv->fdsetindexParsed = true; + priv->memPrealloc = virXPathBoolean("boolean(./memPrealloc)", ctxt) == 1; return 0; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c7125722e0..358f1c163f 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -204,8 +204,9 @@ struct _qemuDomainObjPrivate { /* counter for generating node names for qemu disks */ unsigned long long nodenameindex; - /* counter for generating IDs of fdsets - only relevant during startup */ + /* counter for generating IDs of fdsets */ unsigned int fdsetindex; + bool fdsetindexParsed; /* qemuProcessStartCPUs stores the reason for starting vCPUs here for the * RESUME event handler to use it */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 39ed10ee41..be10d2f3ac 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2261,6 +2261,34 @@ qemuRefreshPRManagerState(virQEMUDriver *driver, } +static int +qemuProcessRefreshFdsetIndex(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(qemuMonitorFdsets) fdsets = NULL; + size_t i; + int rc; + + /* if the previous index was in the status XML we don't need to update it */ + if (priv->fdsetindexParsed) + return 0; + + qemuDomainObjEnterMonitor(priv->driver, vm); + rc = qemuMonitorQueryFdsets(priv->mon, &fdsets); + qemuDomainObjExitMonitor(vm); + + if (rc < 0) + return -1; + + for (i = 0; i < fdsets->nfdsets; i++) { + if (fdsets->fdsets[i].id >= priv->fdsetindex) + priv->fdsetindex = fdsets->fdsets[i].id + 1; + } + + return 0; +} + + static void qemuRefreshRTC(virQEMUDriver *driver, virDomainObj *vm) @@ -8923,6 +8951,9 @@ qemuProcessReconnect(void *opaque) if (qemuRefreshPRManagerState(driver, obj) < 0) goto error; + if (qemuProcessRefreshFdsetIndex(obj) < 0) + goto error; + qemuProcessReconnectCheckMemAliasOrderMismatch(obj); if (qemuConnectAgent(driver, obj) < 0) diff --git a/tests/qemustatusxml2xmldata/backup-pull-in.xml b/tests/qemustatusxml2xmldata/backup-pull-in.xml index 59c934d4f7..e7fdc6c478 100644 --- a/tests/qemustatusxml2xmldata/backup-pull-in.xml +++ b/tests/qemustatusxml2xmldata/backup-pull-in.xml @@ -234,6 +234,7 @@ <chardevStdioLogd/> <allowReboot value='yes'/> <nodename index='0'/> + <fdset index='0'/> <blockjobs active='yes'> <blockjob name='backup-vda-libvirt-3-format' type='backup' state='running' jobflags='0x0'> <disk dst='vda'/> diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml index 52024f242c..b62b3149c2 100644 --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -233,6 +233,7 @@ <chardevStdioLogd/> <allowReboot value='yes'/> <nodename index='0'/> + <fdset index='0'/> <blockjobs active='yes'> <blockjob name='broken-test' type='broken' state='ready' brokentype='commit'/> <blockjob name='commit-vdc-libvirt-9-format' type='commit' state='running' jobflags='0x0'> diff --git a/tests/qemustatusxml2xmldata/blockjob-mirror-in.xml b/tests/qemustatusxml2xmldata/blockjob-mirror-in.xml index 00c8e69adc..73fe437f87 100644 --- a/tests/qemustatusxml2xmldata/blockjob-mirror-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-mirror-in.xml @@ -22,6 +22,7 @@ <libDir path='/tmp'/> <channelTargetDir path='/tmp/channel'/> <allowReboot value='yes'/> + <fdset index='0'/> <blockjobs active='yes'/> <agentTimeout>-2</agentTimeout> <domain type='qemu' id='1'> diff --git a/tests/qemustatusxml2xmldata/migration-in-params-in.xml b/tests/qemustatusxml2xmldata/migration-in-params-in.xml index f4bc5753c4..8b0878c82e 100644 --- a/tests/qemustatusxml2xmldata/migration-in-params-in.xml +++ b/tests/qemustatusxml2xmldata/migration-in-params-in.xml @@ -256,6 +256,7 @@ <channelTargetDir path='/var/lib/libvirt/qemu/channel/target/domain-1-nest'/> <chardevStdioLogd/> <allowReboot value='yes'/> + <fdset index='0'/> <blockjobs active='no'/> <agentTimeout>-2</agentTimeout> <domain type='kvm' id='1'> diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml index c88996f923..7d55db0996 100644 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml @@ -343,6 +343,7 @@ <rememberOwner/> <allowReboot value='yes'/> <nodename index='3'/> + <fdset index='0'/> <blockjobs active='yes'> <blockjob name='drive-virtio-disk0' type='copy' state='ready' jobflags='0x0'> <disk dst='vda'/> diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-out.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-out.xml index 039dcacc58..1a918c0b5a 100644 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-out.xml +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-out.xml @@ -259,6 +259,7 @@ <channelTargetDir path='/var/lib/libvirt/qemu/channel/target/domain-4-upstream'/> <chardevStdioLogd/> <allowReboot value='yes'/> + <fdset index='0'/> <blockjobs active='no'/> <agentTimeout>-2</agentTimeout> <domain type='kvm' id='4'> diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml index 3d1ddd5771..87c67f8300 100644 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml @@ -288,6 +288,7 @@ <channelTargetDir path='/var/lib/libvirt/qemu/channel/target/domain-3-upstream'/> <chardevStdioLogd/> <allowReboot value='yes'/> + <fdset index='0'/> <blockjobs active='no'/> <agentTimeout>-2</agentTimeout> <domain type='kvm' id='3'> diff --git a/tests/qemustatusxml2xmldata/migration-out-params-in.xml b/tests/qemustatusxml2xmldata/migration-out-params-in.xml index cd9dbccd3a..73ac09fb92 100644 --- a/tests/qemustatusxml2xmldata/migration-out-params-in.xml +++ b/tests/qemustatusxml2xmldata/migration-out-params-in.xml @@ -270,6 +270,7 @@ <channelTargetDir path='/var/lib/libvirt/qemu/channel/target/domain-7-nest'/> <chardevStdioLogd/> <allowReboot value='yes'/> + <fdset index='0'/> <blockjobs active='no'/> <agentTimeout>-2</agentTimeout> <domain type='kvm' id='7'> diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index cc5fd1cb74..7759034f7a 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -261,6 +261,7 @@ <chardevStdioLogd/> <allowReboot value='yes'/> <nodename index='123'/> + <fdset index='321'/> <blockjobs active='no'/> <agentTimeout>-2</agentTimeout> <domain type='kvm' id='1'> diff --git a/tests/qemustatusxml2xmldata/upgrade-out.xml b/tests/qemustatusxml2xmldata/upgrade-out.xml index 5218092cb9..ac2ffeddc2 100644 --- a/tests/qemustatusxml2xmldata/upgrade-out.xml +++ b/tests/qemustatusxml2xmldata/upgrade-out.xml @@ -258,6 +258,7 @@ <channelTargetDir path='/var/lib/libvirt/qemu/channel/target/domain-1-upstream'/> <chardevStdioLogd/> <allowReboot value='yes'/> + <fdset index='0'/> <blockjobs active='no'/> <agentTimeout>-2</agentTimeout> <domain type='kvm' id='1'> diff --git a/tests/qemustatusxml2xmldata/vcpus-multi-in.xml b/tests/qemustatusxml2xmldata/vcpus-multi-in.xml index 7f0208453f..0b8bc37c15 100644 --- a/tests/qemustatusxml2xmldata/vcpus-multi-in.xml +++ b/tests/qemustatusxml2xmldata/vcpus-multi-in.xml @@ -308,6 +308,7 @@ <libDir path='/tmp'/> <channelTargetDir path='/tmp/channel'/> <allowReboot value='yes'/> + <fdset index='0'/> <blockjobs active='no'/> <agentTimeout>-2</agentTimeout> <domain type='kvm' id='1729'> -- 2.35.1

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(a)redhat.com&gt; --- src/qemu/qemu_fd.c | 61 +++++++++++++++---------------------- src/qemu/qemu_fd.h | 3 -- tests/qemuhotplugtest.c | 1 + tests/qemumonitorjsontest.c | 7 ----- 4 files changed, 25 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c index 84e9d2bfdf..fe81cc650f 100644 --- a/src/qemu/qemu_fd.c +++ b/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,9 @@ qemuFDPassAddFD(qemuFDPass *fdpass, newfd.opaque = g_strdup_printf("%s%s", fdpass->prefix, NULLSTR_EMPTY(suffix)); + if (!fdpass->useFDSet) + fdpass->path = g_strdup(newfd.opaque); + VIR_APPEND_ELEMENT(fdpass->fds, fdpass->nfds, newfd); } @@ -232,7 +239,6 @@ int qemuFDPassTransferMonitor(qemuFDPass *fdpass, qemuMonitor *mon) { - int fdsetid = -1; size_t i; if (!fdpass) @@ -240,6 +246,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 +268,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 +286,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 diff --git a/src/qemu/qemu_fd.h b/src/qemu/qemu_fd.h index ef35daba8c..d078d4ce5d 100644 --- a/src/qemu/qemu_fd.h +++ b/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); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 21302e0fce..fe8f0b58db 100644 --- a/tests/qemuhotplugtest.c +++ b/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, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index e15c8533f6..c3ee771cbb 100644 --- a/tests/qemumonitorjsontest.c +++ b/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," -- 2.35.1

The only caller doesn't use the fdset info any more. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_fd.c | 5 +---- src/qemu/qemu_monitor.c | 6 ++---- src/qemu/qemu_monitor.h | 7 +------ src/qemu/qemu_monitor_json.c | 37 ++++++------------------------------ src/qemu/qemu_monitor_json.h | 3 +-- 5 files changed, 11 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c index fe81cc650f..cbdf1c938f 100644 --- a/src/qemu/qemu_fd.c +++ b/src/qemu/qemu_fd.c @@ -264,13 +264,10 @@ qemuFDPassTransferMonitor(qemuFDPass *fdpass, for (i = 0; i < fdpass->nfds; i++) { if (fdpass->useFDSet) { - qemuMonitorAddFdInfo fdsetinfo; - if (qemuMonitorAddFileHandleToSet(mon, fdpass->fds[i].fd, fdpass->fdSetID, - fdpass->fds[i].opaque, - &fdsetinfo) < 0) + fdpass->fds[i].opaque) < 0) return -1; } else { if (qemuMonitorSendFileHandle(mon, diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 316cff5b9b..39004201c0 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2506,7 +2506,6 @@ qemuMonitorGraphicsRelocate(qemuMonitor *mon, * @fd: file descriptor to pass to qemu * @fdset: the fdset to register this fd with, -1 to create a new fdset * @opaque: opaque data to associated with this fd - * @info: structure that will be updated with the fd and fdset returned by qemu * * Attempts to register a file descriptor with qemu that can then be referenced * via the file path /dev/fdset/$FDSETID @@ -2515,8 +2514,7 @@ int qemuMonitorAddFileHandleToSet(qemuMonitor *mon, int fd, int fdset, - const char *opaque, - qemuMonitorAddFdInfo *info) + const char *opaque) { VIR_DEBUG("fd=%d,fdset=%i,opaque=%s", fd, fdset, opaque); @@ -2528,7 +2526,7 @@ qemuMonitorAddFileHandleToSet(qemuMonitor *mon, return -1; } - return qemuMonitorJSONAddFileHandleToSet(mon, fd, fdset, opaque, info); + return qemuMonitorJSONAddFileHandleToSet(mon, fd, fdset, opaque); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index e6a50e73f7..79d8486d08 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -941,16 +941,11 @@ int qemuMonitorGraphicsRelocate(qemuMonitor *mon, int tlsPort, const char *tlsSubject); -typedef struct _qemuMonitorAddFdInfo qemuMonitorAddFdInfo; -struct _qemuMonitorAddFdInfo { - int fdset; -}; int qemuMonitorAddFileHandleToSet(qemuMonitor *mon, int fd, int fdset, - const char *opaque, - qemuMonitorAddFdInfo *info); + const char *opaque); int qemuMonitorRemoveFdset(qemuMonitor *mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 6939eaea17..ddef9c6b53 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3594,34 +3594,11 @@ int qemuMonitorJSONGraphicsRelocate(qemuMonitor *mon, } -static int -qemuAddfdInfoParse(virJSONValue *msg, - qemuMonitorAddFdInfo *fdinfo) -{ - virJSONValue *returnObj; - - if (!(returnObj = virJSONValueObjectGetObject(msg, "return"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing or invalid return data in add-fd response")); - return -1; - } - - if (virJSONValueObjectGetNumberInt(returnObj, "fdset-id", &fdinfo->fdset) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing or invalid fdset-id in add-fd response")); - return -1; - } - - return 0; -} - - -/* if fdset is negative, qemu will create a new fdset and add the fd to that */ -int qemuMonitorJSONAddFileHandleToSet(qemuMonitor *mon, - int fd, - int fdset, - const char *opaque, - qemuMonitorAddFdInfo *fdinfo) +int +qemuMonitorJSONAddFileHandleToSet(qemuMonitor *mon, + int fd, + int fdset, + const char *opaque) { g_autoptr(virJSONValue) args = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -3644,12 +3621,10 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitor *mon, if (qemuMonitorJSONCheckError(cmd, reply) < 0) return -1; - if (qemuAddfdInfoParse(reply, fdinfo) < 0) - return -1; - return 0; } + static int qemuMonitorJSONQueryFdsetsParse(virJSONValue *msg, qemuMonitorFdsets **fdsets) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 982fbad44e..3c442d669f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -253,8 +253,7 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitor *mon, int fd, int fdset, - const char *opaque, - qemuMonitorAddFdInfo *info); + const char *opaque); int qemuMonitorJSONRemoveFdset(qemuMonitor *mon, -- 2.35.1

We don't want to keep the FDs open more than we need to. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_fd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c index cbdf1c938f..6e3a7ca491 100644 --- a/src/qemu/qemu_fd.c +++ b/src/qemu/qemu_fd.c @@ -276,6 +276,7 @@ qemuFDPassTransferMonitor(qemuFDPass *fdpass, return -1; } + VIR_FORCE_CLOSE(fdpass->fds[i].fd); fdpass->passed = true; } -- 2.35.1

While the FDs are closed right after use to prevent leaks, at certain point we don't need the whole helper any more. Clear them for char devices after hotplug and on start. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_domain.c | 14 ++++++++++++-- src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_hotplug.c | 3 +++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3032cbc623..95632827cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1526,6 +1526,8 @@ qemuBuildChardevCommand(virCommand *cmd, virCommandAddArgList(cmd, "-chardev", charstr, NULL); + qemuDomainChrSourcePrivateClearFDPass(chrSourcePriv); + return 0; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a726e2624a..2df02943bf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -908,13 +908,23 @@ qemuDomainChrSourcePrivateNew(void) } +void +qemuDomainChrSourcePrivateClearFDPass(qemuDomainChrSourcePrivate *priv) +{ + if (!priv) + return; + + g_clear_pointer(&priv->sourcefd, qemuFDPassFree); + g_clear_pointer(&priv->logfd, qemuFDPassFree); +} + + static void qemuDomainChrSourcePrivateDispose(void *obj) { qemuDomainChrSourcePrivate *priv = obj; - qemuFDPassFree(priv->sourcefd); - qemuFDPassFree(priv->logfd); + qemuDomainChrSourcePrivateClearFDPass(priv); g_free(priv->tlsCertPath); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 358f1c163f..e13c6be399 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -356,6 +356,9 @@ struct _qemuDomainChrSourcePrivate { }; +void +qemuDomainChrSourcePrivateClearFDPass(qemuDomainChrSourcePrivate *priv); + typedef struct _qemuDomainVsockPrivate qemuDomainVsockPrivate; struct _qemuDomainVsockPrivate { virObject parent; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 62cfc29a16..e8e028706b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2293,6 +2293,9 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, if (teardowndevice && qemuDomainNamespaceTeardownChardev(vm, chr) < 0) VIR_WARN("Unable to remove chr device from /dev"); } + + qemuDomainChrSourcePrivateClearFDPass(charpriv); + return ret; exit_monitor: -- 2.35.1

Prepare for the upcoming refactor to use qemuFDPass for all the network related file descriptors: - tapfds - vhostfds - slirp - vdpa This patch adds the private data variables and a utility function to clear it. Clearing is useful since we don't really need the data once the VM is running so we save some memory. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 3 +++ src/qemu/qemu_domain.c | 15 +++++++++++++++ src/qemu/qemu_domain.h | 8 ++++++++ src/qemu/qemu_hotplug.c | 3 +++ 4 files changed, 29 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 95632827cb..0d1517a5d6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8682,6 +8682,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, qemuSlirp *slirp; size_t i; g_autoptr(virJSONValue) hostnetprops = NULL; + qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net); if (qemuDomainValidateActualNetDef(net, qemuCaps) < 0) return -1; @@ -8938,6 +8939,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, ret = 0; cleanup: + qemuDomainNetworkPrivateClearFDs(netpriv); + if (ret < 0) { virErrorPtr saved_err; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2df02943bf..96b0fe56ee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1045,12 +1045,27 @@ qemuDomainNetworkPrivateNew(void) } +void +qemuDomainNetworkPrivateClearFDs(qemuDomainNetworkPrivate *priv) +{ + if (!priv) + return; + + g_clear_pointer(&priv->slirpfd, qemuFDPassFree); + g_clear_pointer(&priv->vdpafd, qemuFDPassFree); + g_slist_free_full(g_steal_pointer(&priv->vhostfds), (GDestroyNotify) qemuFDPassFree); + g_slist_free_full(g_steal_pointer(&priv->tapfds), (GDestroyNotify) qemuFDPassFree); +} + + static void qemuDomainNetworkPrivateDispose(void *obj G_GNUC_UNUSED) { qemuDomainNetworkPrivate *priv = obj; qemuSlirpFree(priv->slirp); + + qemuDomainNetworkPrivateClearFDs(priv); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e13c6be399..85f75ee197 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -398,8 +398,16 @@ struct _qemuDomainNetworkPrivate { virObject parent; qemuSlirp *slirp; + + /* file descriptor transfer helpers */ + qemuFDPass *slirpfd; + GSList *tapfds; + GSList *vhostfds; + qemuFDPass *vdpafd; }; +void +qemuDomainNetworkPrivateClearFDs(qemuDomainNetworkPrivate *priv); typedef enum { QEMU_PROCESS_EVENT_WATCHDOG = 0, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e8e028706b..2923992759 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1181,6 +1181,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, { qemuDomainObjPrivate *priv = vm->privateData; virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_NET, { .net = net } }; + qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net); virErrorPtr originalError = NULL; g_autofree char *slirpfdName = NULL; int slirpfd = -1; @@ -1541,6 +1542,8 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, ret = 0; cleanup: + qemuDomainNetworkPrivateClearFDs(netpriv); + if (ret < 0) { virErrorPreserveLast(&save_err); if (releaseaddr) -- 2.35.1

The helper will aggregate code that is used to connect the network backend to the corresponding host portion. This will be used to refactor the duplicated code between the cold-start and hotplug helper functions. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 43 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 5 +++++ src/qemu/qemu_hotplug.c | 3 +++ 3 files changed, 51 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0d1517a5d6..2c4a1a582a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8652,6 +8652,46 @@ qemuInterfaceVhostuserConnect(virCommand *cmd, return 0; } + +int +qemuBuildInterfaceConnect(virDomainObj *vm G_GNUC_UNUSED, + virDomainNetDef *net, + bool standalone G_GNUC_UNUSED) +{ + virDomainNetType actualType = virDomainNetGetActualType(net); + + switch (actualType) { + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_BRIDGE: + break; + + case VIR_DOMAIN_NET_TYPE_DIRECT: + break; + + case VIR_DOMAIN_NET_TYPE_ETHERNET: + break; + + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + break; + + case VIR_DOMAIN_NET_TYPE_VDPA: + break; + + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_INTERNAL: + case VIR_DOMAIN_NET_TYPE_UDP: + case VIR_DOMAIN_NET_TYPE_LAST: + break; + } + + return 0; +} + + static int qemuBuildInterfaceCommandLine(virQEMUDriver *driver, virDomainObj *vm, @@ -8687,6 +8727,9 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, if (qemuDomainValidateActualNetDef(net, qemuCaps) < 0) return -1; + if (qemuBuildInterfaceConnect(vm, net, standalone) < 0) + return -1; + switch (actualType) { case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a653ff7218..4b4e0bb456 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -94,6 +94,11 @@ qemuBuildHostNetProps(virDomainNetDef *net, const char *slirpfd, const char *vdpadev); +int +qemuBuildInterfaceConnect(virDomainObj *vm, + virDomainNetDef *net, + bool standalone); + /* Current, best practice */ virJSONValue * qemuBuildNicDevProps(virDomainDef *def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2923992759..221815568b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1279,6 +1279,9 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, */ VIR_APPEND_ELEMENT_COPY(vm->def->nets, vm->def->nnets, net); + if (qemuBuildInterfaceConnect(vm, net, false) < 0) + return -1; + switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: -- 2.35.1

Move the setup of the 'vdpa' netdev into the new helper shared between commandline and hotplug code. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 40 +++++++++++++++++++--------------------- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_hotplug.c | 14 ++------------ 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2c4a1a582a..c86ab3f438 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4192,13 +4192,12 @@ qemuBuildHostNetProps(virDomainNetDef *net, size_t tapfdSize, char **vhostfd, size_t vhostfdSize, - const char *slirpfd, - const char *vdpadev) + const char *slirpfd) { bool is_tap = false; virDomainNetType netType = virDomainNetGetActualType(net); size_t i; - + qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net); g_autoptr(virJSONValue) netprops = NULL; if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) { @@ -4333,8 +4332,10 @@ qemuBuildHostNetProps(virDomainNetDef *net, case VIR_DOMAIN_NET_TYPE_VDPA: /* Caller will pass the fd to qemu with add-fd */ - if (virJSONValueObjectAdd(&netprops, "s:type", "vhost-vdpa", NULL) < 0 || - virJSONValueObjectAppendString(netprops, "vhostdev", vdpadev) < 0) + if (virJSONValueObjectAdd(&netprops, + "s:type", "vhost-vdpa", + "s:vhostdev", qemuFDPassGetPath(netpriv->vdpafd), + NULL) < 0) return NULL; if (net->driver.virtio.queues > 1 && @@ -8654,11 +8655,15 @@ qemuInterfaceVhostuserConnect(virCommand *cmd, int -qemuBuildInterfaceConnect(virDomainObj *vm G_GNUC_UNUSED, +qemuBuildInterfaceConnect(virDomainObj *vm, virDomainNetDef *net, bool standalone G_GNUC_UNUSED) { + + qemuDomainObjPrivate *priv = vm->privateData; virDomainNetType actualType = virDomainNetGetActualType(net); + qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net); + VIR_AUTOCLOSE vdpafd = -1; switch (actualType) { case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -8675,6 +8680,11 @@ qemuBuildInterfaceConnect(virDomainObj *vm G_GNUC_UNUSED, break; case VIR_DOMAIN_NET_TYPE_VDPA: + if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0) + return -1; + + netpriv->vdpafd = qemuFDPassNew(net->info.alias, priv); + qemuFDPassAddFD(netpriv->vdpafd, &vdpafd, "-vdpa"); break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: @@ -8703,7 +8713,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, size_t *nnicindexes, int **nicindexes) { - qemuDomainObjPrivate *priv = vm->privateData; virDomainDef *def = vm->def; int ret = -1; g_autoptr(virJSONValue) nicprops = NULL; @@ -8715,7 +8724,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, char **tapfdName = NULL; char **vhostfdName = NULL; g_autofree char *slirpfdName = NULL; - g_autoptr(qemuFDPass) vdpa = NULL; virDomainNetType actualType = virDomainNetGetActualType(net); const virNetDevBandwidth *actualBandwidth; bool requireNicdev = false; @@ -8798,16 +8806,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, break; - case VIR_DOMAIN_NET_TYPE_VDPA: { - VIR_AUTOCLOSE vdpafd = -1; - - if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0) - goto cleanup; - - vdpa = qemuFDPassNew(net->info.alias, priv); - - qemuFDPassAddFD(vdpa, &vdpafd, "-vdpa"); - } + case VIR_DOMAIN_NET_TYPE_VDPA: break; case VIR_DOMAIN_NET_TYPE_USER: @@ -8940,14 +8939,13 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, vhostfd[i] = -1; } - if (qemuFDPassTransferCommand(vdpa, cmd) < 0) + if (qemuFDPassTransferCommand(netpriv->vdpafd, cmd) < 0) return -1; if (!(hostnetprops = qemuBuildHostNetProps(net, tapfdName, tapfdSize, vhostfdName, vhostfdSize, - slirpfdName, - qemuFDPassGetPath(vdpa)))) + slirpfdName))) goto cleanup; if (qemuBuildNetdevCommandlineFromJSON(cmd, hostnetprops, qemuCaps) < 0) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 4b4e0bb456..cae0541445 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -91,8 +91,7 @@ qemuBuildHostNetProps(virDomainNetDef *net, size_t tapfdSize, char **vhostfd, size_t vhostfdSize, - const char *slirpfd, - const char *vdpadev); + const char *slirpfd); int qemuBuildInterfaceConnect(virDomainObj *vm, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 221815568b..d5bdeb83ae 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1185,8 +1185,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, virErrorPtr originalError = NULL; g_autofree char *slirpfdName = NULL; int slirpfd = -1; - int vdpafd = -1; - g_autoptr(qemuFDPass) vdpa = NULL; char **tapfdName = NULL; int *tapfd = NULL; size_t tapfdSize = 0; @@ -1395,13 +1393,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if (qemuDomainAdjustMaxMemLock(vm, false) < 0) goto cleanup; adjustmemlock = true; - - if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0) - goto cleanup; - - vdpa = qemuFDPassNew(net->info.alias, priv); - - qemuFDPassAddFD(vdpa, &vdpafd, "-vdpa"); break; case VIR_DOMAIN_NET_TYPE_SERVER: @@ -1463,7 +1454,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, qemuDomainObjEnterMonitor(driver, vm); - if (qemuFDPassTransferMonitor(vdpa, priv->mon) < 0) { + if (qemuFDPassTransferMonitor(netpriv->vdpafd, priv->mon) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; } @@ -1471,7 +1462,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if (!(netprops = qemuBuildHostNetProps(net, tapfdName, tapfdSize, vhostfdName, vhostfdSize, - slirpfdName, qemuFDPassGetPath(vdpa)))) { + slirpfdName))) { qemuDomainObjExitMonitor(vm); goto cleanup; } @@ -1611,7 +1602,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, VIR_FREE(vhostfdName); virDomainCCWAddressSetFree(ccwaddrs); VIR_FORCE_CLOSE(slirpfd); - VIR_FORCE_CLOSE(vdpafd); return ret; -- 2.35.1

Pre-construct the array the same way for the case when there's only one FD and when there are multiple. We just change the argument name depending on the count. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 66 +++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 19afad22a1..c412bc50e0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4215,54 +4215,50 @@ qemuBuildHostNetProps(virDomainNetDef *net, case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_DIRECT: - case VIR_DOMAIN_NET_TYPE_ETHERNET: - if (virJSONValueObjectAdd(&netprops, "s:type", "tap", NULL) < 0) - return NULL; + case VIR_DOMAIN_NET_TYPE_ETHERNET: { + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + /* for one tapfd/vhostfd 'fd=' shall be used, for more than use 'fds=' */ + const char *tapfd_field = "s:fd"; + g_autofree char *tapfd_arg = NULL; + const char *vhostfd_field = "S:vhostfd"; + g_autofree char *vhostfd_arg = NULL; + bool vhost = false; - /* for one tapfd 'fd=' shall be used, - * for more than one 'fds=' is the right choice */ - if (tapfdSize == 1) { - if (virJSONValueObjectAdd(&netprops, "s:fd", tapfd[0], NULL) < 0) - return NULL; - } else { - g_auto(virBuffer) fdsbuf = VIR_BUFFER_INITIALIZER; + for (i = 0; i < tapfdSize; i++) + virBufferAsprintf(&buf, "%s:", tapfd[i]); - for (i = 0; i < tapfdSize; i++) - virBufferAsprintf(&fdsbuf, "%s:", tapfd[i]); + if (tapfdSize > 1) + tapfd_field = "s:fds"; - virBufferTrim(&fdsbuf, ":"); + virBufferTrim(&buf, ":"); + tapfd_arg = virBufferContentAndReset(&buf); - if (virJSONValueObjectAdd(&netprops, - "s:fds", virBufferCurrentContent(&fdsbuf), - NULL) < 0) - return NULL; - } + if (vhostfdSize > 0) { + vhost = true; - if (vhostfdSize) { - if (virJSONValueObjectAppendBoolean(netprops, "vhost", true) < 0) - return NULL; + for (i = 0; i < vhostfdSize; i++) + virBufferAsprintf(&buf, "%s:", vhostfd[i]); - if (vhostfdSize == 1) { - if (virJSONValueObjectAdd(&netprops, "s:vhostfd", vhostfd[0], NULL) < 0) - return NULL; - } else { - g_auto(virBuffer) fdsbuf = VIR_BUFFER_INITIALIZER; + if (vhostfdSize > 1) + vhostfd_field = "s:vhostfds"; + } - for (i = 0; i < vhostfdSize; i++) - virBufferAsprintf(&fdsbuf, "%s:", vhostfd[i]); + virBufferTrim(&buf, ":"); + vhostfd_arg = virBufferContentAndReset(&buf); - virBufferTrim(&fdsbuf, ":"); + if (virJSONValueObjectAdd(&netprops, + "s:type", "tap", + tapfd_field, tapfd_arg, + "B:vhost", vhost, + vhostfd_field, vhostfd_arg, + NULL) < 0) + return NULL; - if (virJSONValueObjectAdd(&netprops, - "s:vhostfds", virBufferCurrentContent(&fdsbuf), - NULL) < 0) - return NULL; - } - } if (net->tune.sndbuf_specified && virJSONValueObjectAppendNumberUlong(netprops, "sndbuf", net->tune.sndbuf) < 0) return NULL; + } break; case VIR_DOMAIN_NET_TYPE_CLIENT: -- 2.35.1

After the 'qemuFDPass' code was refactored we no longer need to hand off the FD to qemu before we know the path for it. Thus the call to qemuBuildHostNetProps can be moved outside of the monitor critical section. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d5bdeb83ae..525c55baf2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1452,17 +1452,15 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, for (i = 0; i < vhostfdSize; i++) vhostfdName[i] = g_strdup_printf("vhostfd-%s%zu", net->info.alias, i); - qemuDomainObjEnterMonitor(driver, vm); - - if (qemuFDPassTransferMonitor(netpriv->vdpafd, priv->mon) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; - } - if (!(netprops = qemuBuildHostNetProps(net, tapfdName, tapfdSize, vhostfdName, vhostfdSize, - slirpfdName))) { + slirpfdName))) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + + if (qemuFDPassTransferMonitor(netpriv->vdpafd, priv->mon) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; } -- 2.35.1

All callers effectively pass 'net->driver.virtio.queues'. In case of the code in 'qemu_hotplug.c' this value was set to '1' if it was 0 before. Since 'qemuBuildNicDevProps' only uses it if it's greater than 1 we can remove all the extra complexity. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 7 +++---- src/qemu/qemu_command.h | 1 - src/qemu/qemu_hotplug.c | 12 +----------- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fc8e209976..78ea638c26 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4075,7 +4075,6 @@ qemuBuildLegacyNicStr(virDomainNetDef *net) virJSONValue * qemuBuildNicDevProps(virDomainDef *def, virDomainNetDef *net, - size_t vhostfdSize, virQEMUCaps *qemuCaps) { g_autoptr(virJSONValue) props = NULL; @@ -4112,7 +4111,7 @@ qemuBuildNicDevProps(virDomainDef *def, } } - if (vhostfdSize > 1) { + if (net->driver.virtio.queues > 1) { if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { /* ccw provides a one to one relation of fds to queues and * does not support the vectors option @@ -4123,7 +4122,7 @@ qemuBuildNicDevProps(virDomainDef *def, * we should add vectors=2*N+2 where N is the vhostfdSize */ mq = VIR_TRISTATE_SWITCH_ON; - vectors = 2 * vhostfdSize + 2; + vectors = 2 * net->driver.virtio.queues + 2; } } @@ -8987,7 +8986,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, if (qemuCommandAddExtDevice(cmd, &net->info, def, qemuCaps) < 0) goto cleanup; - if (!(nicprops = qemuBuildNicDevProps(def, net, net->driver.virtio.queues, qemuCaps))) + if (!(nicprops = qemuBuildNicDevProps(def, net, qemuCaps))) goto cleanup; if (qemuBuildDeviceCommandlineFromJSON(cmd, nicprops, def, qemuCaps) < 0) goto cleanup; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index cae0541445..3b487a647c 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -102,7 +102,6 @@ qemuBuildInterfaceConnect(virDomainObj *vm, virJSONValue * qemuBuildNicDevProps(virDomainDef *def, virDomainNetDef *net, - size_t vhostfdSize, virQEMUCaps *qemuCaps); char *qemuDeviceDriveHostAlias(virDomainDiskDef *disk); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 28868cf3d0..90ca35bccf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1191,7 +1191,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, char **vhostfdName = NULL; int *vhostfd = NULL; size_t vhostfdSize = 0; - size_t queueSize = 0; g_autoptr(virJSONValue) nicprops = NULL; g_autoptr(virJSONValue) netprops = NULL; int ret = -1; @@ -1287,7 +1286,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, tapfdSize = vhostfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = vhostfdSize = 1; - queueSize = tapfdSize; tapfd = g_new0(int, tapfdSize); memset(tapfd, -1, sizeof(*tapfd) * tapfdSize); vhostfd = g_new0(int, vhostfdSize); @@ -1304,7 +1302,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, tapfdSize = vhostfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = vhostfdSize = 1; - queueSize = tapfdSize; tapfd = g_new0(int, tapfdSize); memset(tapfd, -1, sizeof(*tapfd) * tapfdSize); vhostfd = g_new0(int, vhostfdSize); @@ -1322,7 +1319,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, tapfdSize = vhostfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = vhostfdSize = 1; - queueSize = tapfdSize; tapfd = g_new0(int, tapfdSize); memset(tapfd, -1, sizeof(*tapfd) * tapfdSize); vhostfd = g_new0(int, vhostfdSize); @@ -1336,9 +1332,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - queueSize = net->driver.virtio.queues; - if (!queueSize) - queueSize = 1; if (!qemuDomainSupportsNicdev(vm->def, net)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Nicdev support unavailable")); @@ -1388,9 +1381,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, break; case VIR_DOMAIN_NET_TYPE_VDPA: - queueSize = net->driver.virtio.queues; - if (!queueSize) - queueSize = 1; if (qemuDomainAdjustMaxMemLock(vm, false) < 0) goto cleanup; adjustmemlock = true; @@ -1506,7 +1496,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, for (i = 0; i < vhostfdSize; i++) VIR_FORCE_CLOSE(vhostfd[i]); - if (!(nicprops = qemuBuildNicDevProps(vm->def, net, queueSize, priv->qemuCaps))) + if (!(nicprops = qemuBuildNicDevProps(vm->def, net, priv->qemuCaps))) goto try_remove; qemuDomainObjEnterMonitor(driver, vm); -- 2.35.1

Use the new infrastructure which stores the fds inside 'qemuFDPass' objects in the private data. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 31 +++++++-------------- src/qemu/qemu_hotplug.c | 28 ++++++------------- src/qemu/qemu_interface.c | 58 ++++++++++++++++++--------------------- src/qemu/qemu_interface.h | 6 ++-- tests/qemuxml2argvmock.c | 28 +++++++++++++------ 5 files changed, 67 insertions(+), 84 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 78ea638c26..0b527784c5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8671,23 +8671,27 @@ qemuInterfaceVhostuserConnect(virCommand *cmd, int qemuBuildInterfaceConnect(virDomainObj *vm, virDomainNetDef *net, - bool standalone G_GNUC_UNUSED) + bool standalone) { qemuDomainObjPrivate *priv = vm->privateData; virDomainNetType actualType = virDomainNetGetActualType(net); qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net); VIR_AUTOCLOSE vdpafd = -1; + bool vhostfd = false; switch (actualType) { case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: + vhostfd = true; break; case VIR_DOMAIN_NET_TYPE_DIRECT: + vhostfd = true; break; case VIR_DOMAIN_NET_TYPE_ETHERNET: + vhostfd = true; break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -8712,6 +8716,11 @@ qemuBuildInterfaceConnect(virDomainObj *vm, break; } + if (vhostfd && !standalone) { + if (qemuInterfaceOpenVhostNet(vm, net) < 0) + return -1; + } + return 0; } @@ -8908,26 +8917,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, virNetDevSetMTU(net->ifname, net->mtu) < 0) goto cleanup; - if ((actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || - actualType == VIR_DOMAIN_NET_TYPE_ETHERNET || - actualType == VIR_DOMAIN_NET_TYPE_DIRECT) && - !standalone) { - /* Attempt to use vhost-net mode for these types of - network device */ - vhostfdSize = net->driver.virtio.queues; - if (!vhostfdSize) - vhostfdSize = 1; - - vhostfd = g_new0(int, vhostfdSize); - vhostfdName = g_new0(char *, vhostfdSize); - - memset(vhostfd, -1, vhostfdSize * sizeof(vhostfd[0])); - - if (qemuInterfaceOpenVhostNet(def, net, vhostfd, &vhostfdSize) < 0) - goto cleanup; - } - slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; if (slirp && !standalone) { int slirpfd = qemuSlirpGetFD(slirp); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 90ca35bccf..2bede03e35 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1283,52 +1283,40 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: - tapfdSize = vhostfdSize = net->driver.virtio.queues; + tapfdSize = net->driver.virtio.queues; if (!tapfdSize) - tapfdSize = vhostfdSize = 1; + tapfdSize = 1; tapfd = g_new0(int, tapfdSize); memset(tapfd, -1, sizeof(*tapfd) * tapfdSize); - vhostfd = g_new0(int, vhostfdSize); - memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); if (qemuInterfaceBridgeConnect(vm->def, driver, net, tapfd, &tapfdSize) < 0) goto cleanup; iface_connected = true; - if (qemuInterfaceOpenVhostNet(vm->def, net, vhostfd, &vhostfdSize) < 0) - goto cleanup; break; case VIR_DOMAIN_NET_TYPE_DIRECT: - tapfdSize = vhostfdSize = net->driver.virtio.queues; + tapfdSize = net->driver.virtio.queues; if (!tapfdSize) - tapfdSize = vhostfdSize = 1; + tapfdSize = 1; tapfd = g_new0(int, tapfdSize); memset(tapfd, -1, sizeof(*tapfd) * tapfdSize); - vhostfd = g_new0(int, vhostfdSize); - memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); if (qemuInterfaceDirectConnect(vm->def, driver, net, tapfd, tapfdSize, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) < 0) goto cleanup; iface_connected = true; - if (qemuInterfaceOpenVhostNet(vm->def, net, vhostfd, &vhostfdSize) < 0) - goto cleanup; break; case VIR_DOMAIN_NET_TYPE_ETHERNET: - tapfdSize = vhostfdSize = net->driver.virtio.queues; + tapfdSize = net->driver.virtio.queues; if (!tapfdSize) - tapfdSize = vhostfdSize = 1; + tapfdSize = 1; tapfd = g_new0(int, tapfdSize); memset(tapfd, -1, sizeof(*tapfd) * tapfdSize); - vhostfd = g_new0(int, vhostfdSize); - memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); if (qemuInterfaceEthernetConnect(vm->def, driver, net, tapfd, tapfdSize) < 0) goto cleanup; iface_connected = true; - if (qemuInterfaceOpenVhostNet(vm->def, net, vhostfd, &vhostfdSize) < 0) - goto cleanup; break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -1445,7 +1433,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if (!(netprops = qemuBuildHostNetProps(net, tapfdName, tapfdSize, - vhostfdName, vhostfdSize, + NULL, 0, slirpfdName))) goto cleanup; @@ -1481,7 +1469,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if (qemuMonitorAddNetdev(priv->mon, &netprops, tapfd, tapfdName, tapfdSize, - vhostfd, vhostfdName, vhostfdSize, + NULL, NULL, 0, slirpfd, slirpfdName) < 0) { qemuDomainObjExitMonitor(vm); virDomainAuditNet(vm, NULL, net, "attach", false); diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index e6a26e377f..c807be0745 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -688,49 +688,46 @@ qemuInterfacePrepareSlirp(virQEMUDriver *driver, /** * qemuInterfaceOpenVhostNet: - * @def: domain definition + * @vm: domain object * @net: network definition - * @qemuCaps: qemu binary capabilities - * @vhostfd: array of opened vhost-net device - * @vhostfdSize: number of file descriptors in @vhostfd array * * Open vhost-net, multiple times - if requested. - * In case, no vhost-net is needed, @vhostfdSize is set to 0 - * and 0 is returned. * * Returns: 0 on success * -1 on failure */ int -qemuInterfaceOpenVhostNet(virDomainDef *def, - virDomainNetDef *net, - int *vhostfd, - size_t *vhostfdSize) +qemuInterfaceOpenVhostNet(virDomainObj *vm, + virDomainNetDef *net) { + qemuDomainObjPrivate *priv = vm->privateData; + qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net); size_t i; const char *vhostnet_path = net->backend.vhost; + size_t vhostfdSize = net->driver.virtio.queues; + g_autofree char *prefix = g_strdup_printf("vhostfd-%s", net->info.alias); + + if (!vhostfdSize) + vhostfdSize = 1; if (!vhostnet_path) vhostnet_path = "/dev/vhost-net"; /* If running a plain QEMU guest, or * if the config says explicitly to not use vhost, return now */ - if (def->virtType != VIR_DOMAIN_VIRT_KVM || - net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { - *vhostfdSize = 0; + if (vm->def->virtType != VIR_DOMAIN_VIRT_KVM || + net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) return 0; - } /* If qemu doesn't support vhost-net mode (including the -netdev and * -device command options), don't try to open the device. */ - if (!qemuDomainSupportsNicdev(def, net)) { + if (!qemuDomainSupportsNicdev(vm->def, net)) { if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net is not supported with this QEMU binary")); return -1; } - *vhostfdSize = 0; return 0; } @@ -741,35 +738,34 @@ qemuInterfaceOpenVhostNet(virDomainDef *def, _("vhost-net is only supported for virtio network interfaces")); return -1; } - *vhostfdSize = 0; return 0; } - for (i = 0; i < *vhostfdSize; i++) { - vhostfd[i] = open(vhostnet_path, O_RDWR); + for (i = 0; i < vhostfdSize; i++) { + VIR_AUTOCLOSE fd = open(vhostnet_path, O_RDWR); + g_autoptr(qemuFDPass) pass = qemuFDPassNewDirect(prefix, priv); + g_autofree char *suffix = g_strdup_printf("%zu", i); /* If the config says explicitly to use vhost and we couldn't open it, * report an error. */ - if (vhostfd[i] < 0) { - virDomainAuditNetDevice(def, net, vhostnet_path, false); + if (fd < 0) { + virDomainAuditNetDevice(vm->def, net, vhostnet_path, false); if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net was requested for an interface, but is unavailable")); - goto error; + return -1; } VIR_WARN("Unable to open vhost-net. Opened so far %zu, requested %zu", - i, *vhostfdSize); - *vhostfdSize = i; + i, vhostfdSize); break; } - } - virDomainAuditNetDevice(def, net, vhostnet_path, *vhostfdSize); - return 0; - error: - while (i--) - VIR_FORCE_CLOSE(vhostfd[i]); + qemuFDPassAddFD(pass, &fd, suffix); + netpriv->vhostfds = g_slist_prepend(netpriv->vhostfds, g_steal_pointer(&pass)); + } - return -1; + netpriv->vhostfds = g_slist_reverse(netpriv->vhostfds); + virDomainAuditNetDevice(vm->def, net, vhostnet_path, vhostfdSize); + return 0; } diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h index 438d548065..a566d877b0 100644 --- a/src/qemu/qemu_interface.h +++ b/src/qemu/qemu_interface.h @@ -51,10 +51,8 @@ int qemuInterfaceBridgeConnect(virDomainDef *def, size_t *tapfdSize) ATTRIBUTE_NONNULL(2); -int qemuInterfaceOpenVhostNet(virDomainDef *def, - virDomainNetDef *net, - int *vhostfd, - size_t *vhostfdSize) G_GNUC_NO_INLINE; +int qemuInterfaceOpenVhostNet(virDomainObj *def, + virDomainNetDef *net) G_GNUC_NO_INLINE; int qemuInterfacePrepareSlirp(virQEMUDriver *driver, virDomainNetDef *net, diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 08f176667d..aa82ffa2d6 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -226,20 +226,32 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path G_GNUC_UNUSED, } int -qemuInterfaceOpenVhostNet(virDomainDef *def G_GNUC_UNUSED, - virDomainNetDef *net, - int *vhostfd, - size_t *vhostfdSize) +qemuInterfaceOpenVhostNet(virDomainObj *vm, + virDomainNetDef *net) { + qemuDomainObjPrivate *priv = vm->privateData; + qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net); + g_autofree char *prefix = g_strdup_printf("vhostfd-%s", net->info.alias); + size_t vhostfdSize = net->driver.virtio.queues; size_t i; - if (!virDomainNetIsVirtioModel(net)) { - *vhostfdSize = 0; + if (!vhostfdSize) + vhostfdSize = 1; + + if (!virDomainNetIsVirtioModel(net)) return 0; + + for (i = 0; i < vhostfdSize; i++) { + g_autoptr(qemuFDPass) pass = qemuFDPassNewDirect(prefix, priv); + g_autofree char *suffix = g_strdup_printf("%zu", i); + int fd = STDERR_FILENO + 42 + i; + + qemuFDPassAddFD(pass, &fd, suffix); + netpriv->vhostfds = g_slist_prepend(netpriv->vhostfds, g_steal_pointer(&pass)); } - for (i = 0; i < *vhostfdSize; i++) - vhostfd[i] = STDERR_FILENO + 42 + i; + netpriv->vhostfds = g_slist_reverse(netpriv->vhostfds); + return 0; } -- 2.35.1

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0b527784c5..d30c0de748 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8742,10 +8742,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, g_autofree char *nic = NULL; int *tapfd = NULL; size_t tapfdSize = 0; - int *vhostfd = NULL; - size_t vhostfdSize = 0; char **tapfdName = NULL; - char **vhostfdName = NULL; g_autofree char *slirpfdName = NULL; virDomainNetType actualType = virDomainNetGetActualType(net); const virNetDevBandwidth *actualBandwidth; @@ -8936,13 +8933,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, tapfd[i] = -1; } - for (i = 0; i < vhostfdSize; i++) { - vhostfdName[i] = g_strdup_printf("%d", vhostfd[i]); - virCommandPassFD(cmd, vhostfd[i], - VIR_COMMAND_PASS_FD_CLOSE_PARENT); - vhostfd[i] = -1; - } - for (n = netpriv->tapfds; n; n = n->next) { if (qemuFDPassTransferCommand(n->data, cmd) < 0) return -1; @@ -8958,7 +8948,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, if (!(hostnetprops = qemuBuildHostNetProps(net, tapfdName, tapfdSize, - vhostfdName, vhostfdSize, + NULL, 0, slirpfdName))) goto cleanup; @@ -9003,13 +8993,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, virDomainConfNWFilterTeardown(net); virErrorRestore(&saved_err); } - for (i = 0; vhostfd && i < vhostfdSize; i++) { - if (ret < 0) - VIR_FORCE_CLOSE(vhostfd[i]); - if (vhostfdName) - VIR_FREE(vhostfdName[i]); - } - VIR_FREE(vhostfdName); for (i = 0; tapfd && i < tapfdSize; i++) { if (ret < 0) VIR_FORCE_CLOSE(tapfd[i]); @@ -9017,7 +9000,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, VIR_FREE(tapfdName[i]); } VIR_FREE(tapfdName); - VIR_FREE(vhostfd); VIR_FREE(tapfd); return ret; } -- 2.35.1

All callers now pass NULL/0 as arguments for vhostfd passing so we can remove all the associated code. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 3 +-- src/qemu/qemu_monitor.c | 14 ++------------ src/qemu/qemu_monitor.h | 1 - 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 17fe072e02..f7d3b38a6a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1461,7 +1461,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if (qemuMonitorAddNetdev(priv->mon, &netprops, tapfd, tapfdName, tapfdSize, - NULL, NULL, 0, slirpfd, slirpfdName) < 0) { qemuDomainObjExitMonitor(vm); virDomainAuditNet(vm, NULL, net, "attach", false); @@ -2242,7 +2241,7 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, if (netdevprops) { if (qemuMonitorAddNetdev(priv->mon, &netdevprops, - NULL, NULL, 0, NULL, NULL, 0, -1, NULL) < 0) + NULL, NULL, 0, -1, NULL) < 0) goto exit_monitor; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 39004201c0..57a9e9af62 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2631,17 +2631,15 @@ int qemuMonitorAddNetdev(qemuMonitor *mon, virJSONValue **props, int *tapfd, char **tapfdName, int tapfdSize, - int *vhostfd, char **vhostfdName, int vhostfdSize, int slirpfd, char *slirpfdName) { int ret = -1; - size_t i = 0, j = 0; + size_t i = 0; VIR_DEBUG("props=%p tapfd=%p tapfdName=%p tapfdSize=%d" - "vhostfd=%p vhostfdName=%p vhostfdSize=%d" "slirpfd=%d slirpfdName=%s", props, tapfd, tapfdName, tapfdSize, - vhostfd, vhostfdName, vhostfdSize, slirpfd, slirpfdName); + slirpfd, slirpfdName); QEMU_CHECK_MONITOR(mon); @@ -2649,10 +2647,6 @@ qemuMonitorAddNetdev(qemuMonitor *mon, if (qemuMonitorSendFileHandle(mon, tapfdName[i], tapfd[i]) < 0) goto cleanup; } - for (j = 0; j < vhostfdSize; j++) { - if (qemuMonitorSendFileHandle(mon, vhostfdName[j], vhostfd[j]) < 0) - goto cleanup; - } if (slirpfd > 0 && qemuMonitorSendFileHandle(mon, slirpfdName, slirpfd) < 0) @@ -2666,10 +2660,6 @@ qemuMonitorAddNetdev(qemuMonitor *mon, if (qemuMonitorCloseFileHandle(mon, tapfdName[i]) < 0) VIR_WARN("failed to close device handle '%s'", tapfdName[i]); } - while (j--) { - if (qemuMonitorCloseFileHandle(mon, vhostfdName[j]) < 0) - VIR_WARN("failed to close device handle '%s'", vhostfdName[j]); - } if (qemuMonitorCloseFileHandle(mon, slirpfdName) < 0) VIR_WARN("failed to close device handle '%s'", slirpfdName); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 79d8486d08..1008b33671 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -984,7 +984,6 @@ int qemuMonitorCloseFileHandle(qemuMonitor *mon, int qemuMonitorAddNetdev(qemuMonitor *mon, virJSONValue **props, int *tapfd, char **tapfdName, int tapfdSize, - int *vhostfd, char **vhostfdName, int vhostfdSize, int slirpfd, char *slirpfdName); int qemuMonitorRemoveNetdev(qemuMonitor *mon, -- 2.35.1

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 78baa840a8..a3b4b8c412 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8768,15 +8768,11 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, int ret = -1; g_autoptr(virJSONValue) nicprops = NULL; g_autofree char *nic = NULL; - int *tapfd = NULL; - size_t tapfdSize = 0; - char **tapfdName = NULL; g_autofree char *slirpfdName = NULL; virDomainNetType actualType = virDomainNetGetActualType(net); const virNetDevBandwidth *actualBandwidth; bool requireNicdev = false; qemuSlirp *slirp; - size_t i; g_autoptr(virJSONValue) hostnetprops = NULL; qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net); GSList *n; @@ -8910,17 +8906,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, slirpfdName = g_strdup_printf("%d", slirpfd); } - - for (i = 0; i < tapfdSize; i++) { - if (qemuSecuritySetTapFDLabel(driver->securityManager, - def, tapfd[i]) < 0) - goto cleanup; - tapfdName[i] = g_strdup_printf("%d", tapfd[i]); - virCommandPassFD(cmd, tapfd[i], - VIR_COMMAND_PASS_FD_CLOSE_PARENT); - tapfd[i] = -1; - } - for (n = netpriv->tapfds; n; n = n->next) { if (qemuFDPassTransferCommand(n->data, cmd) < 0) return -1; @@ -8935,7 +8920,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, return -1; if (!(hostnetprops = qemuBuildHostNetProps(net, - tapfdName, tapfdSize, + NULL, 0, slirpfdName))) goto cleanup; @@ -8980,14 +8965,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, virDomainConfNWFilterTeardown(net); virErrorRestore(&saved_err); } - for (i = 0; tapfd && i < tapfdSize; i++) { - if (ret < 0) - VIR_FORCE_CLOSE(tapfd[i]); - if (tapfdName) - VIR_FREE(tapfdName[i]); - } - VIR_FREE(tapfdName); - VIR_FREE(tapfd); return ret; } -- 2.35.1

All callers pass NULL/0. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 9 --------- src/qemu/qemu_command.h | 2 -- src/qemu/qemu_hotplug.c | 1 - 3 files changed, 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a3b4b8c412..92b91b0a52 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4187,8 +4187,6 @@ qemuBuildNicDevProps(virDomainDef *def, virJSONValue * qemuBuildHostNetProps(virDomainNetDef *net, - char **tapfd, - size_t tapfdSize, const char *slirpfd) { virDomainNetType netType = virDomainNetGetActualType(net); @@ -4232,12 +4230,6 @@ qemuBuildHostNetProps(virDomainNetDef *net, if (nfds > 1) tapfd_field = "s:fds"; - } else { - for (i = 0; i < tapfdSize; i++) - virBufferAsprintf(&buf, "%s:", tapfd[i]); - - if (tapfdSize > 1) - tapfd_field = "s:fds"; } virBufferTrim(&buf, ":"); @@ -8920,7 +8912,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, return -1; if (!(hostnetprops = qemuBuildHostNetProps(net, - NULL, 0, slirpfdName))) goto cleanup; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 5a65d94d6f..9bb63d9598 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -87,8 +87,6 @@ qemuBuildChannelGuestfwdNetdevProps(virDomainChrDef *chr); virJSONValue * qemuBuildHostNetProps(virDomainNetDef *net, - char **tapfd, - size_t tapfdSize, const char *slirpfd); int diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c3891ddca7..9f71977a4d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1380,7 +1380,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, goto cleanup; if (!(netprops = qemuBuildHostNetProps(net, - NULL, 0, slirpfdName))) goto cleanup; -- 2.35.1

Both callers populate the variable when qemuInterfacePrepareSlirp returned 1. We can save the hassle in the callers by just doing it right away. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 8 +++----- src/qemu/qemu_interface.c | 10 +++++----- src/qemu/qemu_interface.h | 3 +-- src/qemu/qemu_process.c | 7 +------ 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e126632507..8314d0e546 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1306,14 +1306,12 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if (!priv->disableSlirp && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { qemuSlirp *slirp = NULL; - int rv = qemuInterfacePrepareSlirp(driver, net, &slirp); - if (rv == -1) + if (qemuInterfacePrepareSlirp(driver, net) < 0) goto cleanup; - if (rv == 0) - break; - QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp; + if (!(slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp)) + break; if (qemuSlirpOpen(slirp, driver, vm->def) < 0 || qemuSlirpStart(slirp, vm, driver, net, NULL) < 0) { diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index c807be0745..bda96808eb 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -651,12 +651,12 @@ qemuInterfaceVDPAConnect(virDomainNetDef *net) /* - * Returns: -1 on error, 0 if slirp isn't available, 1 on success + * Returns: -1 on error, 0 on success. Populates net->privateData->slirp if + * the slirp helper is needed. */ int qemuInterfacePrepareSlirp(virQEMUDriver *driver, - virDomainNetDef *net, - qemuSlirp **slirpret) + virDomainNetDef *net) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(qemuSlirp) slirp = NULL; @@ -681,8 +681,8 @@ qemuInterfacePrepareSlirp(virQEMUDriver *driver, return 0; } - *slirpret = g_steal_pointer(&slirp); - return 1; + QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = g_steal_pointer(&slirp); + return 0; } diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h index a566d877b0..e359d4f520 100644 --- a/src/qemu/qemu_interface.h +++ b/src/qemu/qemu_interface.h @@ -55,7 +55,6 @@ int qemuInterfaceOpenVhostNet(virDomainObj *def, virDomainNetDef *net) G_GNUC_NO_INLINE; int qemuInterfacePrepareSlirp(virQEMUDriver *driver, - virDomainNetDef *net, - qemuSlirp **slirp); + virDomainNetDef *net); int qemuInterfaceVDPAConnect(virDomainNetDef *net) G_GNUC_NO_INLINE; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index be10d2f3ac..392a99a769 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5724,13 +5724,8 @@ qemuProcessNetworkPrepareDevices(virQEMUDriver *driver, } else if (actualType == VIR_DOMAIN_NET_TYPE_USER && !priv->disableSlirp && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { - qemuSlirp *slirp = NULL; - int rv = qemuInterfacePrepareSlirp(driver, net, &slirp); - - if (rv == -1) + if (qemuInterfacePrepareSlirp(driver, net) < 0) return -1; - if (rv == 1) - QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp; } } -- 2.35.1

On 5/10/22 10:20 AM, Peter Krempa wrote: Personal taste, perhaps, but the name qemuSlirpStart() implies to me that it is a function that acts on a qemuSlirp* object. With this change, the qemuSlirp object might not even exist when the function is called. I would personally prefer that the function be renamed to reflect this fact. Something like qemuDomainStartSlirp() perhaps? Up to you if you want to change anything. Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt;

No need to ask the callers to call this extra function. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_extdevice.c | 9 --------- src/qemu/qemu_hotplug.c | 3 +-- src/qemu/qemu_slirp.c | 5 ++++- src/qemu/qemu_slirp.h | 4 ---- 4 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 1e54d4ef2c..9ef2a984f5 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -130,7 +130,6 @@ qemuExtDevicesPrepareHost(virQEMUDriver *driver, virDomainObj *vm) { virDomainDef *def = vm->def; - size_t i; if (qemuExtDevicesInitPaths(driver, def) < 0) return -1; @@ -139,14 +138,6 @@ qemuExtDevicesPrepareHost(virQEMUDriver *driver, qemuExtTPMPrepareHost(driver, def) < 0) return -1; - for (i = 0; i < def->nnets; i++) { - virDomainNetDef *net = def->nets[i]; - qemuSlirp *slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; - - if (slirp && qemuSlirpOpen(slirp, driver, def) < 0) - return -1; - } - return 0; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9eeba0210f..8f5e971570 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1313,8 +1313,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if (!(slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp)) break; - if (qemuSlirpOpen(slirp, driver, vm->def) < 0 || - qemuSlirpStart(vm, net, NULL) < 0) { + if (qemuSlirpStart(vm, net, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to start slirp")); goto cleanup; diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c index e1f06573e3..136e4b29d2 100644 --- a/src/qemu/qemu_slirp.c +++ b/src/qemu/qemu_slirp.c @@ -156,7 +156,7 @@ qemuSlirpCreatePidFilename(virQEMUDriverConfig *cfg, } -int +static int qemuSlirpOpen(qemuSlirp *slirp, virQEMUDriver *driver, virDomainDef *def) @@ -272,6 +272,9 @@ qemuSlirpStart(virDomainObj *vm, _("The slirp-helper doesn't support migration")); } + if (qemuSlirpOpen(slirp, driver, vm->def) < 0) + return -1; + if (!(pidfile = qemuSlirpCreatePidFilename(cfg, vm->def, net->info.alias))) return -1; diff --git a/src/qemu/qemu_slirp.h b/src/qemu/qemu_slirp.h index 507ea720fa..5a6a24b178 100644 --- a/src/qemu/qemu_slirp.h +++ b/src/qemu/qemu_slirp.h @@ -57,10 +57,6 @@ void qemuSlirpSetFeature(qemuSlirp *slirp, bool qemuSlirpHasFeature(const qemuSlirp *slirp, qemuSlirpFeature feature); -int qemuSlirpOpen(qemuSlirp *slirp, - virQEMUDriver *driver, - virDomainDef *def); - int qemuSlirpStart(virDomainObj *vm, virDomainNetDef *net, bool incoming); -- 2.35.1

On 5/10/22 10:20 AM, Peter Krempa wrote: If I'm reading this correctly, there's a small behavior change here when 'standalone' is true. Previously we were not passing the slirpfd in that case, but now it looks like we will to pass it regardless of the value of 'standalone'. Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt;

On 5/13/22 6:45 AM, Peter Krempa wrote: OK, I guess I missed that. Also, the name of the 'standalone' argument never really gave me a very good sense of what it was supposed to mean, so I'm in favor of removing it if possible.

None of the callers now uses the slirp fd passing feature, so it can be removed. At this point even the VIR_DEBUG doesn't make sense as it would only log the pointer of 'props'. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 6 ++---- src/qemu/qemu_monitor.c | 23 ++--------------------- src/qemu/qemu_monitor.h | 3 +-- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0f3b1f4bc4..cae7b0dd3b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1401,8 +1401,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, charDevPlugged = true; } - if (qemuMonitorAddNetdev(priv->mon, &netprops, - -1, NULL) < 0) { + if (qemuMonitorAddNetdev(priv->mon, &netprops) < 0) { qemuDomainObjExitMonitor(vm); virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; @@ -2170,8 +2169,7 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, chardevAttached = true; if (netdevprops) { - if (qemuMonitorAddNetdev(priv->mon, &netdevprops, - -1, NULL) < 0) + if (qemuMonitorAddNetdev(priv->mon, &netdevprops) < 0) goto exit_monitor; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a71311d0c4..d44c7f0c60 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2629,31 +2629,12 @@ qemuMonitorCloseFileHandle(qemuMonitor *mon, int qemuMonitorAddNetdev(qemuMonitor *mon, - virJSONValue **props, - int slirpfd, char *slirpfdName) + virJSONValue **props) { - int ret = -1; - - VIR_DEBUG("props=%p " - "slirpfd=%d slirpfdName=%s", - props, - slirpfd, slirpfdName); QEMU_CHECK_MONITOR(mon); - if (slirpfd > 0 && - qemuMonitorSendFileHandle(mon, slirpfdName, slirpfd) < 0) - goto cleanup; - - ret = qemuMonitorJSONAddNetdev(mon, props); - - cleanup: - if (ret < 0) { - if (qemuMonitorCloseFileHandle(mon, slirpfdName) < 0) - VIR_WARN("failed to close device handle '%s'", slirpfdName); - } - - return ret; + return qemuMonitorJSONAddNetdev(mon, props); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index a49ef180c1..b1484fdff8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -982,8 +982,7 @@ int qemuMonitorCloseFileHandle(qemuMonitor *mon, const char *fdname); int qemuMonitorAddNetdev(qemuMonitor *mon, - virJSONValue **props, - int slirpfd, char *slirpfdName); + virJSONValue **props); int qemuMonitorRemoveNetdev(qemuMonitor *mon, const char *alias); -- 2.35.1