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

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

 



On 12/06/16 16:03, Roman Bogorodskiy wrote:
>   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?

Getopt uses an array of strings, so yes, kindof. However, any other parsing option would most likely also need to do this?

>> +        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: OpenPGP digital 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]