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