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) {