On Mon, Apr 24, 2017 at 09:17:12AM +0200, Peter Krempa wrote:
On Sun, Apr 23, 2017 at 20:54:47 -0400, Dan wrote:
Please use your full name for patch submissions.
I just did a new send-email patch submission to the list. Hopefully it corrected my previous mistakes.
Bug 835476 RFE: virsh: add domxml-to-native --domain option (for existing VM) [1]
virsh DOMAIN COMMAND domxml-to-native did not support domain (id|uuid|name) as input for generating hypervisor agent native command, instead only supported XML input from STDIN. Here in this patch, it supports the following syntax: domxml-to-native <format> { [--domain DOMAIN] | [XML] }, i.e., it supports either designating domain (domain id, uuid, or name), or path to XML domain configuration file; NOTE that it deprecated existing STDIN input passing XML functionality. It was tested on the test mock driver and QEMU.
[1]. https://bugzilla.redhat.com/show_bug.cgi?id=835476
Dan
This whole block would get added to the commit message. You should not put anything into it which you don't want there. You want to describe the change briefly, but that's it.
--- tools/virsh-domain.c | 68 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 13 deletions(-)
This patch is corrupted. Usually it's the best to use git-send-email to post patches. Alternatively you can attach the file itself. Pasting the patch into your mail client will corrupt it most probably.
Please re-send a non-corrupted version with your proper name as the author.
I configured my git send-email. Please let me know if there is still format issues.
Also if you didn't make so, make sure you run make check and make syntax-check before posting patches.
This time I ran both of the checks, except that "make check" skipped test.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index db8accfe4..9ac855b19 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -60,6 +60,7 @@ #include "virsh-nodedev.h" #include "viruri.h"
+
Spurious whitespace addition.
I deleted it. Thank you very much for pointing it out.
/* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO # define SA_SIGINFO 0 @@ -9811,9 +9812,13 @@ static const vshCmdOptDef opts_domxmltonative[] = { .flags = VSH_OFLAG_REQ, .help = N_("target config data type format") }, + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ_OPT, + .help = N_("domain name, id or uuid") + }, {.name = "xml", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ, .help = N_("xml data file to export from") }, {.name = NULL} @@ -9822,31 +9827,68 @@ static const vshCmdOptDef opts_domxmltonative[] = { static bool cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) { - bool ret = true; const char *format = NULL; const char *xmlFile = NULL; - char *configData; - char *xmlData; + const char *domain = NULL; + char *configData = NULL; + char *xmlData = NULL; unsigned int flags = 0; + unsigned int domflags = 0; virshControlPtr priv = ctl->privData; + virDomainPtr dom = NULL;
- if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0) + if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0) return false; + + if (vshCommandOptStringReq(ctl, cmd, "domain", &domain) < 0) + return false;
- if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &xmlData) < 0) - return false; + if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0) + return false; + + VSH_EXCLUSIVE_OPTIONS_VAR(domain, xmlFile); + + if (domain) { + domflags = VIRSH_BYID | VIRSH_BYUUID | VIRSH_BYNAME; + dom = virshLookupDomainBy(ctl, domain, domflags);
You can use virshCommandOptDomain instead of this. And the string lookup.
Oh yeah, I used it this time. I would not know its existence without you telling me. So many options to achieve one thing. But this seems to be the most elegant way. Thanks a lot, Dan
+ } + + if (!dom && !xmlFile) {