Re: [PATCH v2 3/5] bhyve: implement virConnectDomainXMLFromNative

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



  Fabian Freyer wrote:

> First, remove escaped newlines and split up the string into an argv-list for
> the bhyve and loader commands, respectively. This is done by iterating over the
> string splitting it by newlines, and then re-iterating over each line,
> splitting it by spaces.
> 
> Since this code reuses part of the code of qemu_parse_command.c
> (in bhyveCommandLine2argv), add the appropriate copyright notices.
> 
> Signed-off-by: Fabian Freyer <fabian.freyer@xxxxxxxxxxxxxxxxxxx>
> ---
>  po/POTFILES.in                  |   1 +
>  src/Makefile.am                 |   2 +
>  src/bhyve/bhyve_driver.c        |  42 +++++++
>  src/bhyve/bhyve_parse_command.c | 263 ++++++++++++++++++++++++++++++++++++++++
>  src/bhyve/bhyve_parse_command.h |  30 +++++
>  5 files changed, 338 insertions(+)
>  create mode 100644 src/bhyve/bhyve_parse_command.c
>  create mode 100644 src/bhyve/bhyve_parse_command.h
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 0d92448..b1580b7 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -15,6 +15,7 @@ src/bhyve/bhyve_command.c
>  src/bhyve/bhyve_device.c
>  src/bhyve/bhyve_driver.c
>  src/bhyve/bhyve_monitor.c
> +src/bhyve/bhyve_parse_command.c
>  src/bhyve/bhyve_process.c
>  src/conf/capabilities.c
>  src/conf/cpu_conf.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 12b66c2..d53c98f 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -901,6 +901,8 @@ BHYVE_DRIVER_SOURCES =						\
>  		bhyve/bhyve_capabilities.h			\
>  		bhyve/bhyve_command.c				\
>  		bhyve/bhyve_command.h				\
> +		bhyve/bhyve_parse_command.c			\
> +		bhyve/bhyve_parse_command.h			\
>  		bhyve/bhyve_device.c				\
>  		bhyve/bhyve_device.h				\
>  		bhyve/bhyve_domain.c				\
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index c4051a1..c7abea4 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -55,6 +55,7 @@
>  #include "bhyve_device.h"
>  #include "bhyve_driver.h"
>  #include "bhyve_command.h"
> +#include "bhyve_parse_command.h"
>  #include "bhyve_domain.h"
>  #include "bhyve_process.h"
>  #include "bhyve_capabilities.h"
> @@ -1536,6 +1537,46 @@ bhyveConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED)
>      return 0;
>  }
>  
> +static char *
> +bhyveConnectDomainXMLFromNative(virConnectPtr conn,
> +                                const char *nativeFormat,
> +                                const char *nativeConfig,
> +                                unsigned int flags)
> +{
> +    char *xml = NULL;
> +    virDomainDefPtr def = NULL;
> +    bhyveConnPtr privconn = conn->privateData;
> +    virCapsPtr capabilities = NULL;
> +    unsigned caps = bhyveDriverGetCaps(conn);
> +
> +    virCheckFlags(0, NULL);
> +
> +    if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0)
> +        goto cleanup;
> +
> +    capabilities = bhyveDriverGetCapabilities(privconn);
> +
> +    if (!capabilities)
> +        goto cleanup;
> +
> +    if (STRNEQ(nativeFormat, BHYVE_CONFIG_FORMAT_ARGV)) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("unsupported config type %s"), nativeFormat);
> +        goto cleanup;
> +    }
> +
> +     def = bhyveParseCommandLineString(nativeConfig, caps, privconn->xmlopt);
> +     if (def == NULL)
> +       goto cleanup;

Nit: this chunk is over-indented by one space.

> +
> +    xml = virDomainDefFormat(def, capabilities, 0);
> +
> + cleanup:
> +    virObjectUnref(capabilities);
> +    virDomainDefFree(def);
> +    return xml;
> +}
> +
>  static virHypervisorDriver bhyveHypervisorDriver = {
>      .name = "bhyve",
>      .connectOpen = bhyveConnectOpen, /* 1.2.2 */
> @@ -1589,6 +1630,7 @@ static virHypervisorDriver bhyveHypervisorDriver = {
>      .connectIsAlive = bhyveConnectIsAlive, /* 1.3.5 */
>      .connectIsSecure = bhyveConnectIsSecure, /* 1.3.5 */
>      .connectIsEncrypted = bhyveConnectIsEncrypted, /* 1.3.5 */
> +    .connectDomainXMLFromNative = bhyveConnectDomainXMLFromNative, /* 1.3.6 */
>  };
>  
>  
> diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
> new file mode 100644
> index 0000000..72367bb
> --- /dev/null
> +++ b/src/bhyve/bhyve_parse_command.c
> @@ -0,0 +1,263 @@
> +/*
> + * bhyve_parse_command.c: Bhyve command parser
> + *
> + * Copyright (C) 2006-2016 Red Hat, Inc.
> + * Copyright (C) 2006 Daniel P. Berrange
> + * Copyright (C) 2016 Fabian Freyer
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Fabian Freyer <fabian.freyer@xxxxxxxxxxxxxxxxxxx>
> + */
> +
> +#include <config.h>
> +
> +#include "bhyve_capabilities.h"
> +#include "bhyve_command.h"
> +#include "bhyve_parse_command.h"
> +#include "viralloc.h"
> +#include "virlog.h"
> +#include "virstring.h"
> +#include "virutil.h"
> +#include "c-ctype.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_BHYVE
> +
> +VIR_LOG_INIT("bhyve.bhyve_parse_command");
> +
> +/*
> + * This function takes a string representation of the command line and removes
> + * all newline characters, if they are prefixed by a backslash. The result
> + * should be a string with one command per line.
> + *
> + * NB: command MUST be NULL-Terminated.
> + */
> +static char *
> +bhyveParseCommandLineUnescape(const char *command)
> +{
> +    size_t len = strlen(command);
> +    char *unescaped = NULL;
> +    char *curr_src = NULL;
> +    char *curr_dst = NULL;
> +
> +    /* Since we are only removing characters, allocating a buffer of the same
> +     * size as command shouldn't be a problem here */
> +    if (VIR_ALLOC_N(unescaped, len) < 0)
> +        return NULL;
> +
> +    /* Iterate over characters in the command, skipping "\\\n", "\\\r" as well
> +     * as "\\\r\n". */
> +    for (curr_src = (char*) command, curr_dst = unescaped; *curr_src != '\0';
> +        curr_src++, curr_dst++) {
> +        if (*curr_src == '\\') {
> +            switch (*(curr_src + 1)) {
> +                case '\n': /* \LF */
> +                    curr_src++;
> +                    curr_dst--;
> +                    break;
> +                case '\r': /* \CR */
> +                    curr_src++;
> +                    curr_dst--;
> +                    if (*curr_src == '\n') /* \CRLF */
> +                        curr_src++;
> +                    break;
> +                default:
> +                    *curr_dst = '\\';
> +            }
> +        }
> +        else *curr_dst = *curr_src;
> +    }
> +
> +    return unescaped;
> +}
> +
> +/*
> + * Try to extract loader and bhyve argv lists from a command line string.
> + */
> +static int
> +bhyveCommandLine2argv(const char *nativeConfig,
                    ^^^^

		    Style: I think it should be 2Argv(...

> +                      int *loader_argc,
> +                      char ***loader_argv,
> +                      int *bhyve_argc,
> +                      char ***bhyve_argv)
> +{
> +    const char *curr = NULL;
> +    char *nativeConfig_unescaped = NULL;
> +    const char *start;
> +    const char *next;
> +    char *line;
> +    char **lines = NULL;
> +    size_t line_count = 0;
> +    size_t lines_alloc = 0;
> +    char **_bhyve_argv = NULL;
> +    char **_loader_argv = NULL;
> +
> +    nativeConfig_unescaped = bhyveParseCommandLineUnescape(nativeConfig);
> +    if (nativeConfig_unescaped == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Failed to unescape command line string"));
> +        goto error;
> +    }
> +
> +    curr = nativeConfig_unescaped;
> +
> +    /* Iterate over string, splitting on sequences of '\n' */
> +    while (curr && *curr != '\0') {
> +        start = curr;
> +        next = strchr(curr, '\n');
> +
> +        if (VIR_STRNDUP(line, curr, next ? next - curr : -1) < 0)
> +            goto error;
> +
> +        if (VIR_RESIZE_N(lines, lines_alloc, line_count, 2) < 0) {
> +            VIR_FREE(line);
> +            goto error;
> +        }
> +
> +        if (*line)
> +            lines[line_count++] = line;
> +        lines[line_count] = NULL;
> +
> +        while (next && (*next == '\n' || *next == '\r'
> +                        || STRPREFIX(next, "\r\n")))
> +            next++;
> +
> +        curr = next;
> +    }
> +
> +    for (int i = 0; i < line_count; i++) {
> +        curr = lines[i];
> +        int j;
> +        char **arglist = NULL;
> +        size_t args_count = 0;
> +        size_t args_alloc = 0;
> +
> +        /* iterate over each line, splitting on sequences of ' '. This code is
> +         * adapted from qemu/qemu_parse_command.c. */

Is that needed for getopt?

> +        while (curr && *curr != '\0') {
> +            char *arg;
> +            start = curr;
> +
> +            if (*start == '\'') {
> +                if (start == curr)
> +                    curr++;
> +                next = strchr(start + 1, '\'');
> +            } else if (*start == '"') {
> +                if (start == curr)
> +                    curr++;
> +                next = strchr(start + 1, '"');
> +            } else {
> +                next = strchr(start, ' ');
> +            }
> +
> +            if (VIR_STRNDUP(arg, curr, next ? next - curr : -1) < 0)
> +                goto error;
> +
> +            if (next && (*next == '\'' || *next == '"'))
> +                next++;
> +
> +            if (VIR_RESIZE_N(arglist, args_alloc, args_count, 2) < 0) {
> +                VIR_FREE(arg);
> +                goto error;
> +            }
> +
> +            arglist[args_count++] = arg;
> +            arglist[args_count] = NULL;
> +
> +            while (next && c_isspace(*next))
> +                next++;
> +
> +            curr = next;
> +        }
> +
> +        /* To prevent a memory leak here, only set the argument lists when
> +         * the first matching command is found. This shouldn't really be a
> +         * problem, since usually no multiple loaders or bhyverun commands
> +         * are specified (this wouldn't really be valid anyways).
> +         * Otherwise, later argument lists may be assigned to _argv without
> +         * freeing the earlier ones. */
> +        if (!_bhyve_argv && STREQ(arglist[0], "/usr/sbin/bhyve")) {
> +            if ((VIR_REALLOC_N(_bhyve_argv, args_count + 1) < 0)
> +                || (!bhyve_argc))
> +                goto error;
> +            for (j = 0; j < args_count; j++)
> +                _bhyve_argv[j] = arglist[j];
> +            _bhyve_argv[j] = NULL;
> +            *bhyve_argc = args_count-1;
> +        }
> +        else if (!_loader_argv) {
> +            if ((VIR_REALLOC_N(_loader_argv, args_count + 1) < 0)
> +                || (!loader_argc))
> +                goto error;
> +            for (j = 0; j < args_count; j++)
> +                _loader_argv[j] = arglist[j];
> +            _loader_argv[j] = NULL;
> +            *loader_argc = args_count-1;
> +        }
> +        /* To prevent a use-after-free here, only free the argument list when
> +         * it is definitely not going to be used */
> +        else
> +            virStringFreeList(arglist);
> +    }
> +
> +    *loader_argv = _loader_argv;
> +    *bhyve_argv = _bhyve_argv;
> +
> +    virStringFreeList(lines);
> +    return 0;
> +
> + error:
> +    VIR_FREE(_loader_argv);
> +    VIR_FREE(_bhyve_argv);
> +    virStringFreeList(lines);
> +    return -1;
> +}
> +
> +virDomainDefPtr
> +bhyveParseCommandLineString(const char* nativeConfig,
> +                            unsigned caps ATTRIBUTE_UNUSED,
> +                            virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED)
> +{
> +    virDomainDefPtr def = NULL;
> +    int bhyve_argc = 0;
> +    char **bhyve_argv = NULL;
> +    int loader_argc = 0;
> +    char **loader_argv = NULL;
> +
> +    if (!(def = virDomainDefNew()))
> +        goto cleanup;
> +
> +    // Initialize defaults.

Nit: I think C++-style comments are not popular in the tree, so probably
will be more consistent to use C-style comments instead.

> +    if (virUUIDGenerate(def->uuid) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("failed to generate uuid"));
> +        goto cleanup;
> +    }
> +    def->id = -1;
> +    def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
> +
> +    if (bhyveCommandLine2argv(nativeConfig,
> +                              &loader_argc, &loader_argv,
> +                              &bhyve_argc, &bhyve_argv)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                    _("Failed to convert the command string to argv-lists.."));
> +        goto cleanup;
> +    }
> +
> +cleanup:
> +    virStringFreeList(loader_argv);
> +    virStringFreeList(bhyve_argv);
> +    return def;
> +}
> diff --git a/src/bhyve/bhyve_parse_command.h b/src/bhyve/bhyve_parse_command.h
> new file mode 100644
> index 0000000..7ffe26c
> --- /dev/null
> +++ b/src/bhyve/bhyve_parse_command.h
> @@ -0,0 +1,30 @@
> +/*
> + * bhyve_parse_command.h: Bhyve command parser
> + *
> + * Copyright (C) 2016 Fabian Freyer
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Fabian Freyer <fabian.freyer@xxxxxxxxxxxxxxxxxxx>
> + */
> +
> +#ifndef __BHYVE_PARSE_COMMAND_H__
> +#define __BHYVE_PARSE_COMMAND_H__
> +
> +virDomainDefPtr bhyveParseCommandLineString(const char* nativeConfig,
> +                                            unsigned caps,
> +                                            virDomainXMLOptionPtr xmlopt);
> +
> +#endif /* __BHYVE_PARSE_COMMAND_H__*/
> -- 
> 2.7.0
> 
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Roman Bogorodskiy

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]