On Tue, Jul 03, 2012 at 16:58:43 +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> The LXC controller code is having to pass around an ever increasing
> number of parameters between methods. To make the code more managable
> introduce a virLXCControllerPtr to hold all this state, starting with
> the container name and virDomainDefPtr object
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> src/lxc/lxc_controller.c | 133 +++++++++++++++++++++++++++++++---------------
> 1 file changed, 91 insertions(+), 42 deletions(-)
>
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index b0e9f46..7447f6c 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -80,6 +80,64 @@ struct cgroup_device_policy {
> };
>
>
> +typedef struct _virLXCController virLXCController;
> +typedef virLXCController *virLXCControllerPtr;
> +struct _virLXCController {
> + char *name;
> + virDomainDefPtr def;
> +};
> +
> +static void virLXCControllerFree(virLXCControllerPtr ctrl);
> +
> +static virLXCControllerPtr virLXCControllerNew(const char *name)
> +{
> + virLXCControllerPtr ctrl = NULL;
> + virCapsPtr caps = NULL;
> + char *configFile = NULL;
> +
> + if (VIR_ALLOC(ctrl) < 0)
> + goto no_memory;
> +
> + if (!(ctrl->name = strdup(name)))
> + goto no_memory;
> +
> + if ((caps = lxcCapsInit(NULL)) == NULL)
> + goto cleanup;
> +
> + if ((configFile = virDomainConfigFile(LXC_STATE_DIR,
> + ctrl->name)) == NULL)
> + goto cleanup;
> +
> + if ((ctrl->def = virDomainDefParseFile(caps,
> + configFile,
> + 1 << VIR_DOMAIN_VIRT_LXC,
> + 0)) == NULL)
> + goto cleanup;
I think all three "goto cleanup"s should rather be goto error...
> +
> +cleanup:
> + VIR_FREE(configFile);
> + virCapabilitiesFree(caps);
> + return ctrl;
> +
> +no_memory:
> + virReportOOMError();
> +//error:
and this label should not be commented out.
> + virLXCControllerFree(ctrl);
> + ctrl = NULL;
> + goto cleanup;
> +}
> +
> +static void virLXCControllerFree(virLXCControllerPtr ctrl)
> +{
> + if (!ctrl)
> + return;
> +
> + virDomainDefFree(ctrl->def);
> + VIR_FREE(ctrl->name);
> +
> + VIR_FREE(ctrl);
> +}
> +
> static int lxcGetLoopFD(char **dev_name)
> {
> int fd = -1;
> @@ -625,12 +683,12 @@ cleanup:
> return rc;
> }
>
> -static char*lxcMonitorPath(virDomainDefPtr def)
> +static char*lxcMonitorPath(virLXCControllerPtr ctrl)
> {
> char *sockpath;
>
> if (virAsprintf(&sockpath, "%s/%s.sock",
> - LXC_STATE_DIR, def->name) < 0)
> + LXC_STATE_DIR, ctrl->def->name) < 0)
> virReportOOMError();
> return sockpath;
> }
> @@ -1362,15 +1420,15 @@ cleanup:
> }
>
> static int
> -lxcControllerRun(virDomainDefPtr def,
> - virSecurityManagerPtr securityDriver,
> - unsigned int nveths,
> - char **veths,
> - int monitor,
> - int client,
> - int *ttyFDs,
> - size_t nttyFDs,
> - int handshakefd)
> +virLXCControllerRun(virLXCControllerPtr ctrl,
> + virSecurityManagerPtr securityDriver,
> + unsigned int nveths,
> + char **veths,
> + int monitor,
> + int client,
> + int *ttyFDs,
> + size_t nttyFDs,
> + int handshakefd)
> {
> int rc = -1;
> int control[2] = { -1, -1};
> @@ -1407,12 +1465,12 @@ lxcControllerRun(virDomainDefPtr def,
> goto cleanup;
> }
>
> - if (lxcSetupLoopDevices(def, &nloopDevs, &loopDevs) < 0)
> + if (lxcSetupLoopDevices(ctrl->def, &nloopDevs, &loopDevs) < 0)
> goto cleanup;
>
> - root = virDomainGetRootFilesystem(def);
> + root = virDomainGetRootFilesystem(ctrl->def);
>
> - if (lxcSetContainerResources(def) < 0)
> + if (lxcSetContainerResources(ctrl->def) < 0)
> goto cleanup;
>
> /*
> @@ -1436,7 +1494,7 @@ lxcControllerRun(virDomainDefPtr def,
> * marked as shared
> */
> if (root) {
> - mount_options = virSecurityManagerGetMountOptions(securityDriver, def);
> + mount_options = virSecurityManagerGetMountOptions(securityDriver, ctrl->def);
> char *opts;
> VIR_DEBUG("Setting up private /dev/pts");
>
> @@ -1525,10 +1583,10 @@ lxcControllerRun(virDomainDefPtr def,
> }
> }
>
> - if (lxcSetPersonality(def) < 0)
> + if (lxcSetPersonality(ctrl->def) < 0)
> goto cleanup;
>
> - if ((container = lxcContainerStart(def,
> + if ((container = lxcContainerStart(ctrl->def,
> securityDriver,
> nveths,
> veths,
> @@ -1618,6 +1676,8 @@ cleanup:
> }
>
>
> +
> +
Looks like a bogus addition of two more empty lines.
> int main(int argc, char *argv[])
> {
> pid_t pid;
> @@ -1629,9 +1689,6 @@ int main(int argc, char *argv[])
> int monitor = -1;
> int handshakefd = -1;
> int bg = 0;
> - virCapsPtr caps = NULL;
> - virDomainDefPtr def = NULL;
> - char *configFile = NULL;
> char *sockpath = NULL;
> const struct option options[] = {
> { "background", 0, NULL, 'b' },
> @@ -1646,6 +1703,7 @@ int main(int argc, char *argv[])
> int *ttyFDs = NULL;
> size_t nttyFDs = 0;
> virSecurityManagerPtr securityDriver = NULL;
> + virLXCControllerPtr ctrl = NULL;
>
> if (setlocale(LC_ALL, "") == NULL ||
> bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
> @@ -1766,31 +1824,22 @@ int main(int argc, char *argv[])
>
> virEventRegisterDefaultImpl();
>
> - if ((caps = lxcCapsInit(NULL)) == NULL)
> - goto cleanup;
> -
> - if ((configFile = virDomainConfigFile(LXC_STATE_DIR,
> - name)) == NULL)
> - goto cleanup;
> -
> - if ((def = virDomainDefParseFile(caps, configFile,
> - 1 << VIR_DOMAIN_VIRT_LXC,
> - 0)) == NULL)
> + if (!(ctrl = virLXCControllerNew(name)))
> goto cleanup;
Oh I see, the wrong labels were caused by cut&pasting the code from here.
>
> VIR_DEBUG("Security model %s type %s label %s imagelabel %s",
> - NULLSTR(def->seclabel.model),
> - virDomainSeclabelTypeToString(def->seclabel.type),
> - NULLSTR(def->seclabel.label),
> - NULLSTR(def->seclabel.imagelabel));
> + NULLSTR(ctrl->def->seclabel.model),
> + virDomainSeclabelTypeToString(ctrl->def->seclabel.type),
> + NULLSTR(ctrl->def->seclabel.label),
> + NULLSTR(ctrl->def->seclabel.imagelabel));
>
> - if (def->nnets != nveths) {
> + if (ctrl->def->nnets != nveths) {
> fprintf(stderr, "%s: expecting %d veths, but got %d\n",
> - argv[0], def->nnets, nveths);
> + argv[0], ctrl->def->nnets, nveths);
> goto cleanup;
> }
>
> - if ((sockpath = lxcMonitorPath(def)) == NULL)
> + if ((sockpath = lxcMonitorPath(ctrl)) == NULL)
> goto cleanup;
>
> if ((monitor = lxcMonitorServer(sockpath)) < 0)
> @@ -1835,17 +1884,17 @@ int main(int argc, char *argv[])
> goto cleanup;
> }
>
> - rc = lxcControllerRun(def, securityDriver,
> - nveths, veths, monitor, client,
> - ttyFDs, nttyFDs, handshakefd);
> + rc = virLXCControllerRun(ctrl, securityDriver,
> + nveths, veths, monitor, client,
> + ttyFDs, nttyFDs, handshakefd);
>
> cleanup:
> - if (def)
> - virPidFileDelete(LXC_STATE_DIR, def->name);
> + virPidFileDelete(LXC_STATE_DIR, name);
> lxcControllerCleanupInterfaces(nveths, veths);
> if (sockpath)
> unlink(sockpath);
> VIR_FREE(sockpath);
> + virLXCControllerFree(ctrl);
>
> return rc ? EXIT_FAILURE : EXIT_SUCCESS;
> }
ACK with the labels fixed.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list