This should resolve https://bugzilla.redhat.com/show_bug.cgi?id=958907 Recent new addition of code to read/write active network state to the NETWORK_STATE_DIR in the network driver broke startup for qemu:///session. The network driver had several state file paths hardcoded to /var, which could never possibly work in session mode. This patch modifies *all* state files to use a variable string that is set differently according to whether or not we're running privileged. There are very definitely other problems preventing dnsmasq and radvd from running in non-privileged mode, but it's more consistent to have the directories used by them be determined in the same fashion. --- src/network/bridge_driver.c | 155 ++++++++++++++++++++++++++++---------------- 1 file changed, 100 insertions(+), 55 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 35202f1..fb00645 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1,4 +1,3 @@ - /* * bridge_driver.c: core driver methods for managing network * @@ -67,11 +66,11 @@ #include "virfile.h" #include "virstring.h" -#define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network" -#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network" +#define NETWORK_PID_DIR "/run/libvirt/network" +#define NETWORK_STATE_DIR "/lib/libvirt/network" -#define DNSMASQ_STATE_DIR LOCALSTATEDIR "/lib/libvirt/dnsmasq" -#define RADVD_STATE_DIR LOCALSTATEDIR "/lib/libvirt/radvd" +#define DNSMASQ_STATE_DIR "/lib/libvirt/dnsmasq" +#define RADVD_STATE_DIR "/lib/libvirt/radvd" #define VIR_FROM_THIS VIR_FROM_NETWORK @@ -84,6 +83,10 @@ struct network_driver { iptablesContext *iptables; char *networkConfigDir; char *networkAutostartDir; + char *stateDir; + char *pidDir; + char *dnsmasqStateDir; + char *radvdStateDir; char *logDir; dnsmasqCapsPtr dnsmasqCaps; }; @@ -133,8 +136,8 @@ networkDnsmasqLeaseFileNameDefault(const char *netname) { char *leasefile; - ignore_value(virAsprintf(&leasefile, DNSMASQ_STATE_DIR "/%s.leases", - netname)); + ignore_value(virAsprintf(&leasefile, "%s/%s.leases", + driverState->dnsmasqStateDir, netname)); return leasefile; } @@ -146,8 +149,8 @@ networkDnsmasqConfigFileName(const char *netname) { char *conffile; - ignore_value(virAsprintf(&conffile, DNSMASQ_STATE_DIR "/%s.conf", - netname)); + ignore_value(virAsprintf(&conffile, "%s/%s.conf", + driverState->dnsmasqStateDir, netname)); return conffile; } @@ -166,8 +169,8 @@ networkRadvdConfigFileName(const char *netname) { char *configfile; - ignore_value(virAsprintf(&configfile, RADVD_STATE_DIR "/%s-radvd.conf", - netname)); + ignore_value(virAsprintf(&configfile, "%s/%s-radvd.conf", + driverState->radvdStateDir, netname)); return configfile; } @@ -187,8 +190,10 @@ networkRemoveInactive(struct network_driver *driver, int ret = -1; /* remove the (possibly) existing dnsmasq and radvd files */ - if (!(dctx = dnsmasqContextNew(def->name, DNSMASQ_STATE_DIR))) + if (!(dctx = dnsmasqContextNew(def->name, + driverState->dnsmasqStateDir))) { goto cleanup; + } if (!(leasefile = networkDnsmasqLeaseFileName(def->name))) goto cleanup; @@ -202,7 +207,8 @@ networkRemoveInactive(struct network_driver *driver, if (!(configfile = networkDnsmasqConfigFileName(def->name))) goto no_memory; - if (!(statusfile = virNetworkConfigFile(NETWORK_STATE_DIR, def->name))) + if (!(statusfile + = virNetworkConfigFile(driverState->stateDir, def->name))) goto no_memory; /* dnsmasq */ @@ -212,7 +218,7 @@ networkRemoveInactive(struct network_driver *driver, /* radvd */ unlink(radvdconfigfile); - virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); + virPidFileDelete(driverState->pidDir, radvdpidbase); /* remove status file */ unlink(statusfile); @@ -279,7 +285,7 @@ networkFindActiveConfigs(struct network_driver *driver) if (obj->def->ips && (obj->def->nips > 0)) { char *radvdpidbase; - ignore_value(virPidFileReadIfAlive(NETWORK_PID_DIR, obj->def->name, + ignore_value(virPidFileReadIfAlive(driverState->pidDir, obj->def->name, &obj->dnsmasqPid, dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps))); @@ -287,7 +293,7 @@ networkFindActiveConfigs(struct network_driver *driver) virReportOOMError(); goto cleanup; } - ignore_value(virPidFileReadIfAlive(NETWORK_PID_DIR, radvdpidbase, + ignore_value(virPidFileReadIfAlive(driverState->pidDir, radvdpidbase, &obj->radvdPid, RADVD)); VIR_FREE(radvdpidbase); } @@ -374,27 +380,59 @@ networkStateInitialize(bool privileged, networkDriverLock(driverState); if (privileged) { - if (virAsprintf(&driverState->logDir, - "%s/log/libvirt/qemu", LOCALSTATEDIR) == -1) - goto out_of_memory; - - if ((base = strdup(SYSCONFDIR "/libvirt")) == NULL) + if (((base = strdup(SYSCONFDIR "/libvirt")) == NULL) || + ((driverState->logDir + = strdup(LOCALSTATEDIR "/log/libvirt/qemu")) == NULL) || + ((driverState->stateDir + = strdup(LOCALSTATEDIR NETWORK_STATE_DIR)) == NULL) || + ((driverState->pidDir + = strdup(LOCALSTATEDIR NETWORK_PID_DIR)) == NULL) || + ((driverState->dnsmasqStateDir + = strdup(LOCALSTATEDIR DNSMASQ_STATE_DIR)) == NULL) || + ((driverState->radvdStateDir + = strdup(LOCALSTATEDIR RADVD_STATE_DIR)) == NULL)) { goto out_of_memory; + } } else { char *userdir = virGetUserCacheDirectory(); if (!userdir) goto error; + userdir = virGetUserConfigDirectory(); + if (virAsprintf(&base, "%s", userdir) < 0) { + VIR_FREE(userdir); + goto out_of_memory; + } + VIR_FREE(userdir); + if (virAsprintf(&driverState->logDir, - "%s/qemu/log", userdir) == -1) { + "%s/qemu/log", userdir) < 0) { VIR_FREE(userdir); goto out_of_memory; } VIR_FREE(userdir); - userdir = virGetUserConfigDirectory(); - if (virAsprintf(&base, "%s", userdir) == -1) { + if (!(userdir = virGetUserRuntimeDirectory())) + goto error; + + if (virAsprintf(&driverState->stateDir, + "%s" NETWORK_STATE_DIR, userdir) < 0) { + VIR_FREE(userdir); + goto out_of_memory; + } + if (virAsprintf(&driverState->pidDir, + "%s" NETWORK_PID_DIR, userdir) < 0) { + VIR_FREE(userdir); + goto out_of_memory; + } + if (virAsprintf(&driverState->dnsmasqStateDir, + "%s" DNSMASQ_STATE_DIR, userdir) < 0) { + VIR_FREE(userdir); + goto out_of_memory; + } + if (virAsprintf(&driverState->radvdStateDir, + "%s" RADVD_STATE_DIR, userdir) < 0) { VIR_FREE(userdir); goto out_of_memory; } @@ -404,12 +442,12 @@ networkStateInitialize(bool privileged, /* Configuration paths are either ~/.libvirt/qemu/... (session) or * /etc/libvirt/qemu/... (system). */ - if (virAsprintf(&driverState->networkConfigDir, "%s/qemu/networks", base) == -1) - goto out_of_memory; - - if (virAsprintf(&driverState->networkAutostartDir, "%s/qemu/networks/autostart", - base) == -1) + if ((virAsprintf(&driverState->networkConfigDir, + "%s/qemu/networks", base) < 0) || + (virAsprintf(&driverState->networkAutostartDir, + "%s/qemu/networks/autostart", base) < 0)) { goto out_of_memory; + } VIR_FREE(base); @@ -421,7 +459,7 @@ networkStateInitialize(bool privileged, driverState->dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ); if (virNetworkLoadAllState(&driverState->networks, - NETWORK_STATE_DIR) < 0) + driverState->stateDir) < 0) goto error; if (virNetworkLoadAllConfigs(&driverState->networks, @@ -489,7 +527,7 @@ networkStateReload(void) { networkDriverLock(driverState); virNetworkLoadAllState(&driverState->networks, - NETWORK_STATE_DIR); + driverState->stateDir); virNetworkLoadAllConfigs(&driverState->networks, driverState->networkConfigDir, driverState->networkAutostartDir); @@ -1060,32 +1098,33 @@ networkStartDhcpDaemon(struct network_driver *driver, goto cleanup; } - if (virFileMakePath(NETWORK_PID_DIR) < 0) { + if (virFileMakePath(driverState->pidDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - NETWORK_PID_DIR); + driverState->pidDir); goto cleanup; } - if (virFileMakePath(NETWORK_STATE_DIR) < 0) { + if (virFileMakePath(driverState->stateDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - NETWORK_STATE_DIR); + driverState->stateDir); goto cleanup; } - if (!(pidfile = virPidFileBuildPath(NETWORK_PID_DIR, network->def->name))) { + if (!(pidfile = virPidFileBuildPath(driverState->pidDir, + network->def->name))) { virReportOOMError(); goto cleanup; } - if (virFileMakePath(DNSMASQ_STATE_DIR) < 0) { + if (virFileMakePath(driverState->dnsmasqStateDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - DNSMASQ_STATE_DIR); + driverState->dnsmasqStateDir); goto cleanup; } - dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); + dctx = dnsmasqContextNew(network->def->name, driverState->dnsmasqStateDir); if (dctx == NULL) goto cleanup; @@ -1113,7 +1152,7 @@ networkStartDhcpDaemon(struct network_driver *driver, * pid */ - ret = virPidFileRead(NETWORK_PID_DIR, network->def->name, + ret = virPidFileRead(driverState->pidDir, network->def->name, &network->dnsmasqPid); if (ret < 0) goto cleanup; @@ -1150,8 +1189,10 @@ networkRefreshDhcpDaemon(struct network_driver *driver, return networkStartDhcpDaemon(driver, network); VIR_INFO("Refreshing dnsmasq for network %s", network->def->bridge); - if (!(dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR))) + if (!(dctx = dnsmasqContextNew(network->def->name, + driverState->dnsmasqStateDir))) { goto cleanup; + } /* Look for first IPv4 address that has dhcp defined. * We only support dhcp-host config on one IPv4 subnetwork @@ -1375,16 +1416,16 @@ networkStartRadvd(struct network_driver *driver ATTRIBUTE_UNUSED, goto cleanup; } - if (virFileMakePath(NETWORK_PID_DIR) < 0) { + if (virFileMakePath(driverState->pidDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - NETWORK_PID_DIR); + driverState->pidDir); goto cleanup; } - if (virFileMakePath(RADVD_STATE_DIR) < 0) { + if (virFileMakePath(driverState->radvdStateDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - RADVD_STATE_DIR); + driverState->radvdStateDir); goto cleanup; } @@ -1393,7 +1434,7 @@ networkStartRadvd(struct network_driver *driver ATTRIBUTE_UNUSED, virReportOOMError(); goto cleanup; } - if (!(pidfile = virPidFileBuildPath(NETWORK_PID_DIR, radvdpidbase))) { + if (!(pidfile = virPidFileBuildPath(driverState->pidDir, radvdpidbase))) { virReportOOMError(); goto cleanup; } @@ -1421,7 +1462,7 @@ networkStartRadvd(struct network_driver *driver ATTRIBUTE_UNUSED, if (virCommandRun(cmd, NULL) < 0) goto cleanup; - if (virPidFileRead(NETWORK_PID_DIR, radvdpidbase, &network->radvdPid) < 0) + if (virPidFileRead(driverState->pidDir, radvdpidbase, &network->radvdPid) < 0) goto cleanup; ret = 0; @@ -1448,7 +1489,7 @@ networkRefreshRadvd(struct network_driver *driver ATTRIBUTE_UNUSED, network->def->name) >= 0) && ((radvdpidbase = networkRadvdPidfileBasename(network->def->name)) != NULL)) { - virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); + virPidFileDelete(driverState->pidDir, radvdpidbase); VIR_FREE(radvdpidbase); } network->radvdPid = -1; @@ -1488,7 +1529,7 @@ networkRestartRadvd(struct network_driver *driver, network->def->name) >= 0) && ((radvdpidbase = networkRadvdPidfileBasename(network->def->name)) != NULL)) { - virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); + virPidFileDelete(driverState->pidDir, radvdpidbase); VIR_FREE(radvdpidbase); } network->radvdPid = -1; @@ -2572,7 +2613,7 @@ static int networkShutdownNetworkVirtual(struct network_driver *driver, if (!(radvdpidbase = networkRadvdPidfileBasename(network->def->name))) { virReportOOMError(); } else { - virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); + virPidFileDelete(driverState->pidDir, radvdpidbase); VIR_FREE(radvdpidbase); } } @@ -2673,7 +2714,8 @@ networkStartNetwork(struct network_driver *driver, /* Persist the live configuration now that anything autogenerated * is setup. */ - if ((ret = virNetworkSaveStatus(NETWORK_STATE_DIR, network)) < 0) { + if ((ret = virNetworkSaveStatus(driverState->stateDir, + network)) < 0) { goto error; } @@ -2703,7 +2745,8 @@ static int networkShutdownNetwork(struct network_driver *driver, if (!virNetworkObjIsActive(network)) return 0; - stateFile = virNetworkConfigFile(NETWORK_STATE_DIR, network->def->name); + stateFile = virNetworkConfigFile(driverState->stateDir, + network->def->name); if (!stateFile) return -1; @@ -3368,8 +3411,10 @@ networkUpdate(virNetworkPtr net, } /* save current network state to disk */ - if ((ret = virNetworkSaveStatus(NETWORK_STATE_DIR, network)) < 0) + if ((ret = virNetworkSaveStatus(driverState->stateDir, + network)) < 0) { goto cleanup; + } } ret = 0; cleanup: @@ -4702,7 +4747,7 @@ networkPlugBandwidth(virNetworkObjPtr net, /* update sum of 'floor'-s of attached NICs */ net->floor_sum += ifaceBand->in->floor; /* update status file */ - if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) { + if (virNetworkSaveStatus(driverState->stateDir, net) < 0) { ignore_value(virBitmapClearBit(net->class_id, class_id)); net->floor_sum -= ifaceBand->in->floor; iface->data.network.actual->class_id = 0; @@ -4748,7 +4793,7 @@ networkUnplugBandwidth(virNetworkObjPtr net, ignore_value(virBitmapClearBit(net->class_id, iface->data.network.actual->class_id)); /* update status file */ - if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) { + if (virNetworkSaveStatus(driverState->stateDir, net) < 0) { net->floor_sum += ifaceBand->in->floor; ignore_value(virBitmapSetBit(net->class_id, iface->data.network.actual->class_id)); -- 1.7.11.7