November 2012 - Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Fix up the default machine type for QMP probing
by Viktor Mihajlovski 30 Nov '12

30 Nov '12

The default machine type must be stored in the first element of the caps->machineTypes array. This was done for help output parsing but not for QMP probing. Added a helper function qemuSetDefaultMachine to apply the same fix up for both probing methods. Further, it was necessary to set caps->nmachineTypes after QMP probing. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 38 ++++++++++++++++++++++++++------------ 1 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d6affb9..6e34cdf 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -292,6 +292,23 @@ qemuCapsProbeCommand(const char *qemu, } +static void +qemuSetDefaultMachine(qemuCapsPtr caps, + size_t defIdx) +{ + char *name = caps->machineTypes[defIdx]; + char *alias = caps->machineAliases[defIdx]; + + memmove(caps->machineTypes + 1, + caps->machineTypes, + sizeof(caps->machineTypes[0]) * defIdx); + memmove(caps->machineAliases + 1, + caps->machineAliases, + sizeof(caps->machineAliases[0]) * defIdx); + caps->machineTypes[0] = name; + caps->machineAliases[0] = alias; +} + /* Format is: * <machine> <desc> [(default)|(alias of <canonical>)] */ @@ -352,18 +369,8 @@ qemuCapsParseMachineTypesStr(const char *output, } while ((p = next)); - if (defIdx != 0) { - char *name = caps->machineTypes[defIdx]; - char *alias = caps->machineAliases[defIdx]; - memmove(caps->machineTypes + 1, - caps->machineTypes, - sizeof(caps->machineTypes[0]) * defIdx); - memmove(caps->machineAliases + 1, - caps->machineAliases, - sizeof(caps->machineAliases[0]) * defIdx); - caps->machineTypes[0] = name; - caps->machineAliases[0] = alias; - } + if (defIdx) + qemuSetDefaultMachine(caps, defIdx); return 0; @@ -2020,6 +2027,7 @@ qemuCapsProbeQMPMachineTypes(qemuCapsPtr caps, int nmachines = 0; int ret = -1; size_t i; + size_t defIdx = 0; if ((nmachines = qemuMonitorGetMachines(mon, &machines)) < 0) goto cleanup; @@ -2049,7 +2057,13 @@ qemuCapsProbeQMPMachineTypes(qemuCapsPtr caps, goto cleanup; } } + if (machines[i]->isDefault) + defIdx = i; } + caps->nmachineTypes = nmachines; + + if (defIdx) + qemuSetDefaultMachine(caps, defIdx); ret = 0; -- 1.7.0.4

2 2

[libvirt] [PATCH v2] Introduce APIs for splitting/joining strings
by Daniel P. Berrange 30 Nov '12

30 Nov '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> This introduces a few new APIs for dealing with strings. One to split a char * into a char **, another to join a char ** into a char *, and finally one to free a char ** There is a simple test suite to validate the edge cases too. No more need to use the horrible strtok_r() API, or hand-written code for splitting strings. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- .gitignore | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 6 ++ src/util/virstring.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virstring.h | 38 +++++++++++ tests/Makefile.am | 6 ++ tests/virstringtest.c | 162 +++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 382 insertions(+) create mode 100644 src/util/virstring.c create mode 100644 src/util/virstring.h create mode 100644 tests/virstringtest.c diff --git a/.gitignore b/.gitignore index 12fbe0e..0dadd21 100644 --- a/.gitignore +++ b/.gitignore @@ -170,6 +170,7 @@ /tests/virlockspacetest /tests/virnet*test /tests/virshtest +/tests/virstringtest /tests/virtimetest /tests/viruritest /tests/vmx2xmltest diff --git a/src/Makefile.am b/src/Makefile.am index 6401dec..b5c20c8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -112,6 +112,7 @@ UTIL_SOURCES = \ util/virnetlink.c util/virnetlink.h \ util/virrandom.h util/virrandom.c \ util/virsocketaddr.h util/virsocketaddr.c \ + util/virstring.h util/virstring.c \ util/virtime.h util/virtime.c \ util/viruri.h util/viruri.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 93a21cc..08974d0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1812,6 +1812,12 @@ virSetErrorLogPriorityFunc; virStrerror; +# virstring.h +virStringSplit; +virStringJoin; +virStringFreeList; + + # virtime.h virTimeFieldsNow; virTimeFieldsNowRaw; diff --git a/src/util/virstring.c b/src/util/virstring.c new file mode 100644 index 0000000..1917e9a --- /dev/null +++ b/src/util/virstring.c @@ -0,0 +1,168 @@ +/* + * Copyright (C) 2012 Red Hat, Inc. + * + * 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/>. + * + * Authors: + * Daniel P. Berrange <berrange(a)redhat.com> + */ + +#include <config.h> + +#include "virstring.h" +#include "memory.h" +#include "buf.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/* + * The following virStringSplit & virStringJoin methods + * are derived from g_strsplit / g_strjoin in glib2, + * also available under the LGPLv2+ license terms + */ + +/** + * virStringSplit: + * @string: a string to split + * @delim: a string which specifies the places at which to split + * the string. The delimiter is not included in any of the resulting + * strings, unless @max_tokens is reached. + * @max_tokens: the maximum number of pieces to split @string into. + * If this is 0, the string is split completely. + * + * Splits a string into a maximum of @max_tokens pieces, using the given + * @delim. If @max_tokens is reached, the remainder of @string is + * appended to the last token. + * + * As a special case, the result of splitting the empty string "" is an empty + * vector, not a vector containing a single string. The reason for this + * special case is that being able to represent a empty vector is typically + * more useful than consistent handling of empty elements. If you do need + * to represent empty elements, you'll need to check for the empty string + * before calling virStringSplit(). + * + * Return value: a newly-allocated NULL-terminated array of strings. Use + * virStringFreeList() to free it. + */ +char **virStringSplit(const char *string, + const char *delim, + size_t max_tokens) +{ + char **tokens = NULL; + size_t ntokens = 0; + size_t maxtokens = 0; + const char *remainder = string; + char *tmp; + size_t i; + + if (max_tokens == 0) + max_tokens = INT_MAX; + + tmp = strstr(remainder, delim); + if (tmp) { + size_t delimlen = strlen(delim); + + while (--max_tokens && tmp) { + size_t len = tmp - remainder; + + if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0) + goto no_memory; + + if (!(tokens[ntokens] = strndup(remainder, len))) + goto no_memory; + ntokens++; + remainder = tmp + delimlen; + tmp = strstr(remainder, delim); + } + } + if (*string) { + if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0) + goto no_memory; + + if (!(tokens[ntokens] = strdup(remainder))) + goto no_memory; + ntokens++; + } + + if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0) + goto no_memory; + tokens[ntokens++] = NULL; + + return tokens; + +no_memory: + virReportOOMError(); + for (i = 0 ; i < ntokens ; i++) + VIR_FREE(tokens[i]); + VIR_FREE(tokens); + return NULL; +} + + +/** + * virStringJoin: + * @strings: a NULL-terminated array of strings to join + * @delim: a string to insert between each of the strings + * + * Joins a number of strings together to form one long string, with the + * @delim inserted between each of them. The returned string + * should be freed with VIR_FREE(). + * + * Returns: a newly-allocated string containing all of the strings joined + * together, with @delim between them + */ +char *virStringJoin(const char **strings, + const char *delim) +{ + char *ret; + virBuffer buf = VIR_BUFFER_INITIALIZER; + while (*strings) { + virBufferAdd(&buf, *strings, -1); + if (*(strings+1)) + virBufferAdd(&buf, delim, -1); + strings++; + } + if (virBufferError(&buf)) { + virReportOOMError(); + return NULL; + } + ret = virBufferContentAndReset(&buf); + if (!ret) { + if (!(ret = strdup(""))) { + virReportOOMError(); + return NULL; + } + } + return ret; +} + + +/** + * virStringFreeList: + * @str_array: a NULL-terminated array of strings to free + * + * Frees a NULL-terminated array of strings, and the array itself. + * If called on a NULL value, virStringFreeList() simply returns. + */ +void virStringFreeList(char **strings) +{ + char **tmp = strings; + while (tmp && *tmp) { + VIR_FREE(*tmp); + tmp++; + } + VIR_FREE(strings); +} diff --git a/src/util/virstring.h b/src/util/virstring.h new file mode 100644 index 0000000..a569fe0 --- /dev/null +++ b/src/util/virstring.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2007-2012 Red Hat, Inc. + * + * 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/>. + * + * Authors: + * Daniel P. Berrange <berrange(a)redhat.com> + */ + +#ifndef __VIR_STRING_H__ +# define __VIR_STRING_H__ + +# include "internal.h" + +char **virStringSplit(const char *string, + const char *delim, + size_t max_tokens) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +char *virStringJoin(const char **strings, + const char *delim) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +void virStringFreeList(char **strings); + +#endif /* __VIR_STRING_H__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 18f5b51..8435e1a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -95,6 +95,7 @@ test_programs = virshtest sockettest \ virauthconfigtest \ virbitmaptest \ virlockspacetest \ + virstringtest \ $(NULL) if WITH_SECDRIVER_SELINUX @@ -539,6 +540,11 @@ virtimetest_SOURCES = \ virtimetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) virtimetest_LDADD = $(LDADDS) +virstringtest_SOURCES = \ + virstringtest.c testutils.h testutils.c +virstringtest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) +virstringtest_LDADD = $(LDADDS) + virlockspacetest_SOURCES = \ virlockspacetest.c testutils.h testutils.c virlockspacetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) diff --git a/tests/virstringtest.c b/tests/virstringtest.c new file mode 100644 index 0000000..0b3e7c5 --- /dev/null +++ b/tests/virstringtest.c @@ -0,0 +1,162 @@ +/* + * Copyright (C) 2012 Red Hat, Inc. + * + * 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: Daniel P. Berrange <berrange(a)redhat.com> + */ + +#include <config.h> + +#include <stdlib.h> +#include <signal.h> + +#include "testutils.h" +#include "util.h" +#include "virterror_internal.h" +#include "memory.h" +#include "logging.h" + +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct testSplitData { + const char *string; + const char *delim; + size_t max_tokens; + const char **tokens; +}; + + +struct testJoinData { + const char *string; + const char *delim; + const char **tokens; +}; + +static int testSplit(const void *args) +{ + const struct testSplitData *data = args; + char **got; + char **tmp1; + const char **tmp2; + int ret = -1; + + if (!(got = virStringSplit(data->string, data->delim, data->max_tokens))) { + VIR_DEBUG("Got no tokens at all"); + return -1; + } + + tmp1 = got; + tmp2 = data->tokens; + while (*tmp1 && *tmp2) { + if (STRNEQ(*tmp1, *tmp2)) { + fprintf(stderr, "Mismatch '%s' vs '%s'\n", *tmp1, *tmp2); + goto cleanup; + } + tmp1++; + tmp2++; + } + if (*tmp1) { + fprintf(stderr, "Too many pieces returned\n"); + goto cleanup; + } + if (*tmp2) { + fprintf(stderr, "Too few pieces returned\n"); + goto cleanup; + } + + ret = 0; +cleanup: + virStringFreeList(got); + + return ret; +} + + +static int testJoin(const void *args) +{ + const struct testJoinData *data = args; + char *got; + int ret = -1; + + if (!(got = virStringJoin(data->tokens, data->delim))) { + VIR_DEBUG("Got no result"); + return -1; + } + if (STRNEQ(got, data->string)) { + fprintf(stderr, "Mismatch '%s' vs '%s'\n", got, data->string); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(got); + + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + +#define TEST_SPLIT(str, del, max, toks) \ + do { \ + struct testSplitData splitData = { \ + .string = str, \ + .delim = del, \ + .max_tokens = max, \ + .tokens = toks, \ + }; \ + struct testJoinData joinData = { \ + .string = str, \ + .delim = del, \ + .tokens = toks, \ + }; \ + if (virtTestRun("Split " #str, 1, testSplit, &splitData) < 0) \ + ret = -1; \ + if (virtTestRun("Join " #str, 1, testJoin, &joinData) < 0) \ + ret = -1; \ + } while (0) + + const char *tokens1[] = { NULL }; + TEST_SPLIT("", " ", 0, tokens1); + + const char *tokens2[] = { "", "", NULL }; + TEST_SPLIT(" ", " ", 0, tokens2); + + const char *tokens3[] = { "", "", "", NULL }; + TEST_SPLIT(" ", " ", 0, tokens3); + + const char *tokens4[] = { "The", "quick", "brown", "fox", NULL }; + TEST_SPLIT("The quick brown fox", " ", 0, tokens4); + + const char *tokens5[] = { "The quick ", " fox", NULL }; + TEST_SPLIT("The quick brown fox", "brown", 0, tokens5); + + const char *tokens6[] = { "", "The", "quick", "brown", "fox", NULL }; + TEST_SPLIT(" The quick brown fox", " ", 0, tokens6); + + const char *tokens7[] = { "The", "quick", "brown", "fox", "", NULL }; + TEST_SPLIT("The quick brown fox ", " ", 0, tokens7); + + + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.7.11.7

2 1

[libvirt] [PATCH] Allow duration=0 for virsh nodesuspend
by Daniel P. Berrange 30 Nov '12

30 Nov '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> The virNodeSuspend API allows for a duration of 0, to mean no timed wakup. virsh needlessly forbids this though Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- tools/virsh-domain.c | 34 ++++++++++++++++++++-------------- tools/virsh-host.c | 2 +- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3ca246b..9f1a3d4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4030,8 +4030,8 @@ static const vshCmdOptDef opts_shutdown[] = { static bool cmdShutdown(vshControl *ctl, const vshCmd *cmd) { - virDomainPtr dom; - bool ret = true; + virDomainPtr dom = NULL; + bool ret = false; const char *name; const char *mode = NULL; int flags = 0; @@ -4062,14 +4062,13 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) } else { vshError(ctl, _("Unknown mode %s value, expecting " "'acpi', 'agent', 'initctl' or 'signal'"), mode); - return false; + goto cleanup; } tmp++; } - virStringFreeList(modes); if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) - return false; + goto cleanup; if (flags) rv = virDomainShutdownFlags(dom, flags); @@ -4079,10 +4078,14 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, _("Domain %s is being shutdown\n"), name); } else { vshError(ctl, _("Failed to shutdown domain %s"), name); - ret = false; + goto cleanup; } - virDomainFree(dom); + ret = true; +cleanup: + if (dom) + virDomainFree(dom); + virStringFreeList(modes); return ret; } @@ -4104,8 +4107,8 @@ static const vshCmdOptDef opts_reboot[] = { static bool cmdReboot(vshControl *ctl, const vshCmd *cmd) { - virDomainPtr dom; - bool ret = true; + virDomainPtr dom = NULL; + bool ret = false; const char *name; const char *mode = NULL; int flags = 0; @@ -4135,23 +4138,26 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) } else { vshError(ctl, _("Unknown mode %s value, expecting " "'acpi', 'agent', 'initctl' or 'signal'"), mode); - return false; + goto cleanup; } tmp++; } - virStringFreeList(modes); if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) - return false; + goto cleanup; if (virDomainReboot(dom, flags) == 0) { vshPrint(ctl, _("Domain %s is being rebooted\n"), name); } else { vshError(ctl, _("Failed to reboot domain %s"), name); - ret = false; + goto cleanup; } - virDomainFree(dom); + ret = true; +cleanup: + if (dom) + virDomainFree(dom); + virStringFreeList(modes); return ret; } diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 3701f56..3d13e01 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -537,7 +537,7 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd) return false; } - if (duration <= 0) { + if (duration < 0) { vshError(ctl, "%s", _("Invalid duration")); return false; } -- 1.7.11.7

2 1

[libvirt] [PATCH] Allow comma separated list of shutdown/reboot modes with virsh
by Daniel P. Berrange 30 Nov '12

30 Nov '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> The shutdown and reboot commands in virsh allow a comma separated list of mode values Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- tools/virsh-domain.c | 25 +++++++++++++++++++++++-- tools/virsh.pod | 16 ++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a7ffd2b..3ca246b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -48,6 +48,7 @@ #include "virfile.h" #include "virkeycode.h" #include "virmacaddr.h" +#include "virstring.h" #include "virsh-domain-monitor.h" #include "virterror_internal.h" #include "virtypedparam.h" @@ -4035,13 +4036,21 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) const char *mode = NULL; int flags = 0; int rv; + char **modes, **tmp; if (vshCommandOptString(cmd, "mode", &mode) < 0) { vshError(ctl, "%s", _("Invalid type")); return false; } - if (mode) { + if (!(modes = virStringSplit(mode, ",", 0))) { + vshError(ctl, "%s", _("Cannot parse mode string")); + return false; + } + + tmp = modes; + while (*tmp) { + mode = *tmp; if (STREQ(mode, "acpi")) { flags |= VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN; } else if (STREQ(mode, "agent")) { @@ -4055,7 +4064,9 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) "'acpi', 'agent', 'initctl' or 'signal'"), mode); return false; } + tmp++; } + virStringFreeList(modes); if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; @@ -4098,13 +4109,21 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) const char *name; const char *mode = NULL; int flags = 0; + char **modes, **tmp; if (vshCommandOptString(cmd, "mode", &mode) < 0) { vshError(ctl, "%s", _("Invalid type")); return false; } - if (mode) { + if (!(modes = virStringSplit(mode, ",", 0))) { + vshError(ctl, "%s", _("Cannot parse mode string")); + return false; + } + + tmp = modes; + while (*tmp) { + mode = *tmp; if (STREQ(mode, "acpi")) { flags |= VIR_DOMAIN_REBOOT_ACPI_POWER_BTN; } else if (STREQ(mode, "agent")) { @@ -4118,7 +4137,9 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) "'acpi', 'agent', 'initctl' or 'signal'"), mode); return false; } + tmp++; } + virStringFreeList(modes); if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; diff --git a/tools/virsh.pod b/tools/virsh.pod index c901b11..7dde3df 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1137,7 +1137,7 @@ If I<--live> is specified, set scheduler information of a running guest. If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. -=item B<reboot> I<domain> [I<--mode acpi|agent>] +=item B<reboot> I<domain> [I<--mode MODE-LIST>] Reboot a domain. This acts just as if the domain had the B<reboot> command run from the console. The command returns as soon as it has @@ -1149,7 +1149,11 @@ I<on_reboot> parameter in the domain's XML definition. By default the hypervisor will try to pick a suitable shutdown method. To specify an alternative method, the I<--mode> parameter -can specify C<acpi> or C<agent>. +can specify a comma separated list which includes C<acpi>, C<agent>, +C<initctl> and C<signal>. The order in which drivers will try each +mode is undefined, and not related to the order specified to virsh. +For strict control over ordering, use a single mode at a time and +repeat the command. =item B<reset> I<domain> @@ -1567,7 +1571,7 @@ The I<--maximum> flag controls the maximum number of virtual cpus that can be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag. -=item B<shutdown> I<domain> [I<--mode acpi|agent>] +=item B<shutdown> I<domain> [I<--mode MODE-LIST>] Gracefully shuts down a domain. This coordinates with the domain OS to perform graceful shutdown, so there is no guarantee that it will @@ -1584,7 +1588,11 @@ snapshot metadata with B<snapshot-create>. By default the hypervisor will try to pick a suitable shutdown method. To specify an alternative method, the I<--mode> parameter -can specify C<acpi> or C<agent>. +can specify a comma separated list which includes C<acpi>, C<agent>, +C<initctl> and C<signal>. The order in which drivers will try each +mode is undefined, and not related to the order specified to virsh. +For strict control over ordering, use a single mode at a time and +repeat the command. =item B<start> I<domain-name-or-uuid> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] -- 1.7.11.7

2 5

[libvirt] [PATCH] Ensure virDomainShutdownFlags logs flags parameter
by Daniel P. Berrange 30 Nov '12

30 Nov '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> Pushed under trivial rule Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/libvirt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libvirt.c b/src/libvirt.c index 757bfa8..955e761 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3246,7 +3246,7 @@ virDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain); + VIR_DOMAIN_DEBUG(domain, "flags=%u", flags); virResetLastError(); -- 1.7.11.7

1 0

[libvirt] [PATCH] Quote client identity in SASL whitelist log message
by Daniel P. Berrange 30 Nov '12

30 Nov '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> When seeing a message virNetSASLContextCheckIdentity:146 : SASL client admin not allowed in whitelist it isn't immediately obvious that 'admin' is the identity being checked. Quote the string to make it more obvious Pushed under trivial rule --- src/rpc/virnetsaslcontext.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c index 3001871..b6b68d5 100644 --- a/src/rpc/virnetsaslcontext.c +++ b/src/rpc/virnetsaslcontext.c @@ -163,7 +163,7 @@ int virNetSASLContextCheckIdentity(virNetSASLContextPtr ctxt, } /* Denied */ - VIR_ERROR(_("SASL client %s not allowed in whitelist"), identity); + VIR_ERROR(_("SASL client identity '%s' not allowed in whitelist"), identity); /* This is the most common error: make it informative. */ virReportError(VIR_ERR_SYSTEM_ERROR, "%s", -- 1.7.11.7

1 0

[libvirt] [PATCH] Fix uninitialized variables
by Guido Günther 30 Nov '12

30 Nov '12

detecet by http://honk.sigxcpu.org:8001/job/libvirt-build/348/console --- Probably o.k. to push under the build breaker rule but I'd better check. Cheers, -- Guido src/qemu/qemu_monitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c1f7c41..f85bb76 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2408,7 +2408,7 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, int vlan, const char *netname) { - int ret; + int ret = -1; VIR_DEBUG("mon=%p netname=%s", mon, netname); @@ -2541,7 +2541,7 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon, mon, drivestr, controllerAddr->domain, controllerAddr->bus, controllerAddr->slot, controllerAddr->function); - int ret; + int ret = 1; if (!mon) { virReportError(VIR_ERR_INVALID_ARG, "%s", -- 1.7.10.4

3 3

[libvirt] [PATCH] Introduce APIs for splitting/joining strings
by Daniel P. Berrange 30 Nov '12

30 Nov '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> This introduces a few new APIs for dealing with strings. One to split a char * into a char **, another to join a char ** into a char *, and finally one to free a char ** There is a simple test suite to validate the edge cases too. No more need to use the horrible strtok_r() API, or hand-written code for splitting strings. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/Makefile.am | 1 + src/libvirt_private.syms | 6 ++ src/util/virstring.c | 129 +++++++++++++++++++++++++++++++++++++ src/util/virstring.h | 36 +++++++++++ tests/Makefile.am | 6 ++ tests/virstringtest.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 342 insertions(+) create mode 100644 src/util/virstring.c create mode 100644 src/util/virstring.h create mode 100644 tests/virstringtest.c diff --git a/src/Makefile.am b/src/Makefile.am index 6401dec..b5c20c8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -112,6 +112,7 @@ UTIL_SOURCES = \ util/virnetlink.c util/virnetlink.h \ util/virrandom.h util/virrandom.c \ util/virsocketaddr.h util/virsocketaddr.c \ + util/virstring.h util/virstring.c \ util/virtime.h util/virtime.c \ util/viruri.h util/viruri.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 93a21cc..08974d0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1812,6 +1812,12 @@ virSetErrorLogPriorityFunc; virStrerror; +# virstring.h +virStringSplit; +virStringJoin; +virStringFreeList; + + # virtime.h virTimeFieldsNow; virTimeFieldsNowRaw; diff --git a/src/util/virstring.c b/src/util/virstring.c new file mode 100644 index 0000000..97dddac --- /dev/null +++ b/src/util/virstring.c @@ -0,0 +1,129 @@ +/* + * Copyright (C) 2007-2012 Red Hat, Inc. + * + * 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/>. + * + * Authors: + * Daniel P. Berrange <berrange(a)redhat.com> + */ + +#include <config.h> + +#include "virstring.h" +#include "memory.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +char **virStringSplit(const char *string, + const char *delim, + size_t max_tokens) +{ + char **tokens = NULL; + size_t ntokens = 0; + size_t maxtokens = 0; + const char *remainder = string; + char *tmp; + size_t i; + + if (max_tokens < 1) + max_tokens = INT_MAX; + + tmp = strstr(remainder, delim); + if (tmp) { + size_t delimlen = strlen(delim); + + while (--max_tokens && tmp) { + size_t len = tmp - remainder; + + if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0) + goto no_memory; + + if (!(tokens[ntokens] = strndup(remainder, len))) + goto no_memory; + ntokens++; + remainder = tmp + delimlen; + tmp = strstr(remainder, delim); + } + } + if (*string) { + if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0) + goto no_memory; + + if (!(tokens[ntokens] = strdup(remainder))) + goto no_memory; + ntokens++; + } + + if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0) + goto no_memory; + tokens[ntokens++] = NULL; + + return tokens; + +no_memory: + virReportOOMError(); + for (i = 0 ; i < ntokens ; i++) { + VIR_FREE(tokens[i]); + } + VIR_FREE(tokens); + return NULL; +} + + +char *virStringJoin(const char **strings, + const char *delim) +{ + size_t len = 0; + size_t delimlen = strlen(delim); + const char **tmp = strings; + char *string; + char *offset; + + while (tmp && *tmp) { + len += strlen(*tmp); + len += delimlen; + tmp++; + } + + if (VIR_ALLOC_N(string, len + 1) < 0) { + virReportOOMError(); + return NULL; + } + + tmp = strings; + offset = string; + while (tmp && *tmp) { + offset = stpcpy(offset, *tmp); + if (*(tmp+1)) + offset = stpcpy(offset, delim); + len += strlen(*tmp); + len += delimlen; + tmp++; + } + + return string; +} + + +void virStringFreeList(char **strings) +{ + char **tmp = strings; + while (tmp && *tmp) { + VIR_FREE(*tmp); + tmp++; + } + VIR_FREE(strings); +} diff --git a/src/util/virstring.h b/src/util/virstring.h new file mode 100644 index 0000000..14c77dc --- /dev/null +++ b/src/util/virstring.h @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2007-2012 Red Hat, Inc. + * + * 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/>. + * + * Authors: + * Daniel P. Berrange <berrange(a)redhat.com> + */ + +#ifndef __VIR_STRING_H__ +# define __VIR_STRING_H__ + +# include "internal.h" + +char **virStringSplit(const char *string, + const char *delim, + size_t max_tokens); + +char *virStringJoin(const char **strings, + const char *delim); + +void virStringFreeList(char **strings); + +#endif /* __VIR_STRING_H__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 18f5b51..8435e1a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -95,6 +95,7 @@ test_programs = virshtest sockettest \ virauthconfigtest \ virbitmaptest \ virlockspacetest \ + virstringtest \ $(NULL) if WITH_SECDRIVER_SELINUX @@ -539,6 +540,11 @@ virtimetest_SOURCES = \ virtimetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) virtimetest_LDADD = $(LDADDS) +virstringtest_SOURCES = \ + virstringtest.c testutils.h testutils.c +virstringtest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) +virstringtest_LDADD = $(LDADDS) + virlockspacetest_SOURCES = \ virlockspacetest.c testutils.h testutils.c virlockspacetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) diff --git a/tests/virstringtest.c b/tests/virstringtest.c new file mode 100644 index 0000000..de050d6 --- /dev/null +++ b/tests/virstringtest.c @@ -0,0 +1,164 @@ +/* + * Copyright (C) 2011 Red Hat, Inc. + * + * 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: Daniel P. Berrange <berrange(a)redhat.com> + */ + +#include <config.h> + +#include <stdlib.h> +#include <signal.h> + +#include "testutils.h" +#include "util.h" +#include "virterror_internal.h" +#include "memory.h" +#include "logging.h" + +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct testSplitData { + const char *string; + const char *delim; + size_t max_tokens; + const char **tokens; +}; + + +struct testJoinData { + const char *string; + const char *delim; + const char **tokens; +}; + +static int testSplit(const void *args) +{ + const struct testSplitData *data = args; + char **got; + char **tmp1; + const char **tmp2; + int ret = -1; + + if (!(got = virStringSplit(data->string, data->delim, data->max_tokens))) { + VIR_DEBUG("Got no tokens at all"); + return -1; + } + + tmp1 = got; + tmp2 = data->tokens; + while (*tmp1 && *tmp2) { + if (STRNEQ(*tmp1, *tmp2)) { + fprintf(stderr, "Mismatch '%s' vs '%s'\n", *tmp1, *tmp2); + goto cleanup; + } + tmp1++; + tmp2++; + } + if (*tmp1) { + fprintf(stderr, "Too many pieces returned\n"); + goto cleanup; + } + if (*tmp2) { + fprintf(stderr, "Too few pieces returned\n"); + goto cleanup; + } + + ret = 0; +cleanup: + virStringFreeList(got); + + return ret; +} + + +static int testJoin(const void *args) +{ + const struct testJoinData *data = args; + char *got; + int ret = -1; + + if (!(got = virStringJoin(data->tokens, data->delim))) { + VIR_DEBUG("Got no result"); + return -1; + } + if (STRNEQ(got, data->string)) { + fprintf(stderr, "Mismatch '%s' vs '%s'\n", got, data->string); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(got); + + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + + signal(SIGPIPE, SIG_IGN); + +#define TEST_SPLIT(str, del, max, toks) \ + do { \ + struct testSplitData splitData = { \ + .string = str, \ + .delim = del, \ + .max_tokens = max, \ + .tokens = toks, \ + }; \ + struct testJoinData joinData = { \ + .string = str, \ + .delim = del, \ + .tokens = toks, \ + }; \ + if (virtTestRun("Split " #str, 1, testSplit, &splitData) < 0) \ + ret = -1; \ + if (virtTestRun("Join " #str, 1, testJoin, &joinData) < 0) \ + ret = -1; \ + } while (0) + + const char *tokens1[] = { NULL }; + TEST_SPLIT("", " ", 0, tokens1); + + const char *tokens2[] = { "", "", NULL }; + TEST_SPLIT(" ", " ", 0, tokens2); + + const char *tokens3[] = { "", "", "", NULL }; + TEST_SPLIT(" ", " ", 0, tokens3); + + const char *tokens4[] = { "The", "quick", "brown", "fox", NULL }; + TEST_SPLIT("The quick brown fox", " ", 0, tokens4); + + const char *tokens5[] = { "The quick ", " fox", NULL }; + TEST_SPLIT("The quick brown fox", "brown", 0, tokens5); + + const char *tokens6[] = { "", "The", "quick", "brown", "fox", NULL }; + TEST_SPLIT(" The quick brown fox", " ", 0, tokens6); + + const char *tokens7[] = { "The", "quick", "brown", "fox", "", NULL }; + TEST_SPLIT("The quick brown fox ", " ", 0, tokens7); + + + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.7.11.7

2 4

[libvirt] [PATCH] qemu: don't attempt undefined QMP commands
by Eric Blake 30 Nov '12

30 Nov '12

https://bugzilla.redhat.com/show_bug.cgi?id=872292 Libvirt should not attempt to call a QMP command that has not been documented in qemu.git - if future qemu introduces a command by the same name but with subtly different semantics, then libvirt will be broken when trying to use that command. See also this attempt to convert the three snapshot commands to QMP: https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg01597.html although it looks like that will still not happen before qemu 1.3. That thread eventually decided that qemu would use the name 'save-vm' rather than 'savevm', which mitigates the fact that libvirt's attempt to use a QMP 'savevm' would be broken, but we might not be as lucky on the other commands. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONSetCPU) (qemuMonitorJSONAddDrive, qemuMonitorJSONDriveDel) (qemuMonitorJSONCreateSnapshot, qemuMonitorJSONLoadSnapshot) (qemuMonitorJSONDeleteSnapshot): Use only HMP fallback for now. (qemuMonitorJSONAddHostNetwork, qemuMonitorJSONRemoveHostNetwork) (qemuMonitorJSONAttachDrive, qemuMonitorJSONGetGuestDriveAddress): Delete; QMP implies QEMU_CAPS_DEVICE, which prefers AddNetdev, RemoveNetdev, and AddDrive anyways. * src/qemu/qemu_monitor.c (qemuMonitorAddHostNetwork) (qemuMonitorRemoveHostNetwork, qemuMonitorAttachDrive): Reflect deleted commands. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONAddHostNetwork) (qemuMonitorJSONRemoveHostNetwork, qemuMonitorJSONAttachDrive): Likewise. --- src/qemu/qemu_monitor.c | 9 +- src/qemu/qemu_monitor_json.c | 311 ++++-------------------------------------- src/qemu/qemu_monitor_json.h | 12 -- 3 files changed, 31 insertions(+), 301 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index aef5044..43e45ef 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2387,7 +2387,8 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONAddHostNetwork(mon, netstr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_add")); else ret = qemuMonitorTextAddHostNetwork(mon, netstr); @@ -2418,7 +2419,8 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONRemoveHostNetwork(mon, vlan, netname); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_del")); else ret = qemuMonitorTextRemoveHostNetwork(mon, vlan, netname); return ret; @@ -2548,7 +2550,8 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONAttachDrive(mon, drivestr, controllerAddr, driveAddr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using AddDrive")); else ret = qemuMonitorTextAttachDrive(mon, drivestr, controllerAddr, driveAddr); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9fcd7e8..195d89f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2094,44 +2094,9 @@ cleanup: int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, int cpu, int online) { - int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("cpu_set", - "U:cpu", (unsigned long long)cpu, - "s:state", online ? "online" : "offline", - NULL); - virJSONValuePtr reply = NULL; - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("cpu_set command not found, trying HMP"); - ret = qemuMonitorTextSetCPU(mon, cpu, online); - goto cleanup; - } - - if (ret == 0) { - /* XXX See if CPU soft-failed due to lack of ACPI */ -#if 0 - if (qemuMonitorJSONHasError(reply, "DeviceNotActive") || - qemuMonitorJSONHasError(reply, "KVMMissingCap")) - goto cleanup; -#endif - - /* See if any other fatal error occurred */ - ret = qemuMonitorJSONCheckError(cmd, reply); - - /* Real success */ - if (ret == 0) - ret = 1; - } - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for cpu hotplug */ + VIR_DEBUG("no QMP support for cpu_set, trying HMP"); + return qemuMonitorTextSetCPU(mon, cpu, online); } @@ -2674,52 +2639,6 @@ int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon, } -int qemuMonitorJSONAddHostNetwork(qemuMonitorPtr mon, - const char *netstr) -{ - int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("host_net_add", - "s:device", netstr, - NULL); - virJSONValuePtr reply = NULL; - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; -} - - -int qemuMonitorJSONRemoveHostNetwork(qemuMonitorPtr mon, - int vlan, - const char *netname) -{ - int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("host_net_remove", - "i:vlan", vlan, - "s:device", netname, - NULL); - virJSONValuePtr reply = NULL; - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; -} - - int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, const char *netdevstr) { @@ -2886,74 +2805,6 @@ int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } -static int -qemuMonitorJSONGetGuestDriveAddress(virJSONValuePtr reply, - virDomainDeviceDriveAddress *driveAddr) -{ - virJSONValuePtr addr; - - addr = virJSONValueObjectGet(reply, "return"); - if (!addr || addr->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("drive_add reply was missing device address")); - return -1; - } - - if (virJSONValueObjectGetNumberUint(addr, "bus", &driveAddr->bus) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("drive_add reply was missing device bus number")); - return -1; - } - - if (virJSONValueObjectGetNumberUint(addr, "unit", &driveAddr->unit) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("drive_add reply was missing device unit number")); - return -1; - } - - return 0; -} - - -int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon, - const char *drivestr, - virDevicePCIAddress* controllerAddr, - virDomainDeviceDriveAddress* driveAddr) -{ - int ret; - virJSONValuePtr cmd = NULL; - virJSONValuePtr reply = NULL; - char *dev; - - if (virAsprintf(&dev, "%.2x:%.2x.%.1x", - controllerAddr->bus, controllerAddr->slot, controllerAddr->function) < 0) { - virReportOOMError(); - return -1; - } - - cmd = qemuMonitorJSONMakeCommand("drive_add", - "s:pci_addr", dev, - "s:opts", drivestr, - NULL); - VIR_FREE(dev); - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret == 0 && - qemuMonitorJSONGetGuestDriveAddress(reply, driveAddr) < 0) - ret = -1; - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; -} - - int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuMonitorPCIAddress **addrs ATTRIBUTE_UNUSED) { @@ -3025,32 +2876,9 @@ cleanup: int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("drive_add", - "s:pci_addr", "dummy", - "s:opts", drivestr, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply) < 0)) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("drive_add command not found, trying HMP"); - ret = qemuMonitorTextAddDrive(mon, drivestr); - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for drive_add */ + VIR_DEBUG("drive_add command not found, trying HMP"); + return qemuMonitorTextAddDrive(mon, drivestr); } @@ -3058,42 +2886,19 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon, const char *drivestr) { int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - VIR_DEBUG("JSONDriveDel drivestr=%s", drivestr); - cmd = qemuMonitorJSONMakeCommand("drive_del", - "s:id", drivestr, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("drive_del command not found, trying HMP"); - if ((ret = qemuMonitorTextDriveDel(mon, drivestr)) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->code == VIR_ERR_OPERATION_UNSUPPORTED) { - VIR_ERROR("%s", - _("deleting disk is not supported. " - "This may leak data if disk is reassigned")); - ret = 1; - virResetLastError(); - } + /* XXX Update to use QMP, if QMP ever adds support for drive_del */ + VIR_DEBUG("drive_del command not found, trying HMP"); + if ((ret = qemuMonitorTextDriveDel(mon, drivestr)) < 0) { + virErrorPtr err = virGetLastError(); + if (err && err->code == VIR_ERR_OPERATION_UNSUPPORTED) { + VIR_ERROR("%s", + _("deleting disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + virResetLastError(); } - } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { - /* NB: device not found errors mean the drive was - * auto-deleted and we ignore the error */ - ret = 0; - } else { - ret = qemuMonitorJSONCheckError(cmd, reply); } - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -3131,89 +2936,23 @@ int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, int qemuMonitorJSONCreateSnapshot(qemuMonitorPtr mon, const char *name) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("savevm", - "s:name", name, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("savevm command not found, trying HMP"); - ret = qemuMonitorTextCreateSnapshot(mon, name); - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for savevm */ + VIR_DEBUG("savevm command not found, trying HMP"); + return qemuMonitorTextCreateSnapshot(mon, name); } int qemuMonitorJSONLoadSnapshot(qemuMonitorPtr mon, const char *name) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("loadvm", - "s:name", name, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("loadvm command not found, trying HMP"); - ret = qemuMonitorTextLoadSnapshot(mon, name); - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for loadvm */ + VIR_DEBUG("loadvm command not found, trying HMP"); + return qemuMonitorTextLoadSnapshot(mon, name); } int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("delvm", - "s:name", name, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("delvm command not found, trying HMP"); - ret = qemuMonitorTextDeleteSnapshot(mon, name); - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for delvm */ + VIR_DEBUG("delvm command not found, trying HMP"); + return qemuMonitorTextDeleteSnapshot(mon, name); } int diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c62ae24..acca4ec 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -179,13 +179,6 @@ int qemuMonitorJSONSendFileHandle(qemuMonitorPtr mon, int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon, const char *fdname); -int qemuMonitorJSONAddHostNetwork(qemuMonitorPtr mon, - const char *netstr); - -int qemuMonitorJSONRemoveHostNetwork(qemuMonitorPtr mon, - int vlan, - const char *netname); - int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, const char *netdevstr); @@ -199,11 +192,6 @@ int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, virDevicePCIAddress *guestAddr); -int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon, - const char *drivestr, - virDevicePCIAddress *controllerAddr, - virDomainDeviceDriveAddress *driveAddr); - int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon, qemuMonitorPCIAddress **addrs); -- 1.7.1

3 4

[libvirt] [PATCH] storage: fix scsi detach regression with cgroup ACLs
by Eric Blake 30 Nov '12

30 Nov '12

https://bugzilla.redhat.com/show_bug.cgi?id=876828 Commit 38c4a9cc introduced a regression in hot unplugging of disks from qemu, where cgroup device ACLs were no longer being revoked (thankfully not a security hole: cgroup ACLs only prevent open() of the disk; so reverting the ACL prevents future abuse but doesn't stop abuse from an fd that was already opened before the ACL change). Commit 1b2ebf95 overlooked that there were two spots affected. * src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Transfer backing chain before deletion. * src/qemu/qemu_driver.c (qemuDomainDetachDeviceDiskLive): Fix spacing (partly to ensure a different-looking patch). --- I blame git for letting me find this - I did a 'pull --rebase' on top of libvirt.git, and noticed that my working patch was still on the tree - it turns out that the hunk for qemu_hotplug.c is _identical_ except for the context of the function name needing a fix. I still wish git would be more vocal when it finds an alternate place to apply a patch when function names don't match. src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c37bdb9..ae98dbf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6070,7 +6070,7 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver, if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) ret = qemuDomainDetachPciDiskDevice(driver, vm, dev); else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) - ret = qemuDomainDetachDiskDevice(driver, vm, dev); + ret = qemuDomainDetachDiskDevice(driver, vm, dev); else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) ret = qemuDomainDetachDiskDevice(driver, vm, dev); else diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cfeae68..2d4a822 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2091,6 +2091,8 @@ int qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, virDomainDiskRemove(vm->def, i); + dev->data.disk->backingChain = detach->backingChain; + detach->backingChain = NULL; virDomainDiskDefFree(detach); if (virSecurityManagerRestoreImageLabel(driver->securityManager, -- 1.7.1

2 2