On 5/10/22 10:20 AM, Peter Krempa wrote:
The 'driver' can be taken from the private data of
'vm' and 'slirp' can
be taken from private data of 'net', both of which we need anyways.
Additionally by checking whether slirp needs to be started inside the
function we don't need to do this logic in the callers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_extdevice.c | 4 +---
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_slirp.c | 10 +++++++---
src/qemu/qemu_slirp.h | 4 +---
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 537b130394..1e54d4ef2c 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -185,10 +185,8 @@ qemuExtDevicesStart(virQEMUDriver *driver,
for (i = 0; i < def->nnets; i++) {
virDomainNetDef *net = def->nets[i];
- qemuSlirp *slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
- if (slirp &&
- qemuSlirpStart(slirp, vm, driver, net, incomingMigration) < 0)
+ if (qemuSlirpStart(vm, net, incomingMigration) < 0)
return -1;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8314d0e546..9eeba0210f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1314,7 +1314,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
break;
if (qemuSlirpOpen(slirp, driver, vm->def) < 0 ||
- qemuSlirpStart(slirp, vm, driver, net, NULL) < 0) {
+ 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 c832cfc20b..e1f06573e3 100644
--- a/src/qemu/qemu_slirp.c
+++ b/src/qemu/qemu_slirp.c
@@ -245,12 +245,13 @@ qemuSlirpSetupCgroup(qemuSlirp *slirp,
int
-qemuSlirpStart(qemuSlirp *slirp,
- virDomainObj *vm,
- virQEMUDriver *driver,
+qemuSlirpStart(virDomainObj *vm,
virDomainNetDef *net,
bool incoming)
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>
{
+ qemuDomainObjPrivate *priv = vm->privateData;
+ virQEMUDriver *driver = priv->driver;
+ qemuSlirp *slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autoptr(virCommand) cmd = NULL;
g_autofree char *pidfile = NULL;
@@ -262,6 +263,9 @@ qemuSlirpStart(qemuSlirp *slirp,
VIR_AUTOCLOSE errfd = -1;
bool killDBusDaemon = false;
+ if (!slirp)
+ return 0;
+
if (incoming &&
!qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/qemu/qemu_slirp.h b/src/qemu/qemu_slirp.h
index a9a09cd5f8..507ea720fa 100644
--- a/src/qemu/qemu_slirp.h
+++ b/src/qemu/qemu_slirp.h
@@ -61,9 +61,7 @@ int qemuSlirpOpen(qemuSlirp *slirp,
virQEMUDriver *driver,
virDomainDef *def);
-int qemuSlirpStart(qemuSlirp *slirp,
- virDomainObj *vm,
- virQEMUDriver *driver,
+int qemuSlirpStart(virDomainObj *vm,
virDomainNetDef *net,
bool incoming);