Re: [libvirt] [PATCH] RFE: virsh: add domxml-to-native <fmt> [--domain DOMAIN] option

On Sun, Apr 23, 2017 at 20:54:47 -0400, Dan wrote: Please use your full name for patch submissions.

> 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.

Also if you didn't make so, make sure you run make check and make syntax-check before posting patches.

> > 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.

> /* 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.

> + } > + > + if (!dom && !xmlFile) {