[libvirt] [PATCH v4] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND
The option allows someone to run domain-to-native on already existing domain without the need of supplying their XML. It is basically wrapper around 'virsh dumpxml | virsh domxml-to-native /dev/stdin'. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476 Signed-off-by: Daniel Liu <srwx4096@gmail.com> --- tools/virsh-domain.c | 51 ++++++++++++++++++++++++++++++++++++++------------- tools/virsh.pod | 7 ++++--- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ccb514ef9..3496359c5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9848,9 +9848,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} @@ -9859,30 +9863,51 @@ static const vshCmdOptDef opts_domxmltonative[] = { static bool cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) { - bool ret = true; + bool ret = false; const char *format = NULL; - const char *xmlFile = NULL; - char *configData; - char *xmlData; + const char *xml = NULL; + char *xmlData = NULL; + char *configData = NULL; unsigned int flags = 0; virshControlPtr priv = ctl->privData; + virDomainPtr dom = NULL; if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0) + vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0) return false; - if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &xmlData) < 0) - return false; + VSH_EXCLUSIVE_OPTIONS("domain", "xml"); - configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags); - if (configData != NULL) { - vshPrint(ctl, "%s", configData); - VIR_FREE(configData); + if (vshCommandOptBool(cmd, "domain") && + (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))) + return false; + + if (dom) { + xmlData = virDomainGetXMLDesc(dom, flags); + } else if (xml) { + if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlData) < 0) + goto cleanup; } else { - ret = false; + vshError(ctl, "%s", _("need either domain or domain XML")); + goto cleanup; + } + + if (!xmlData) { + vshError(ctl, "%s", _("failed to retrieve XML")); + goto cleanup; } + if (!(configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags))) { + goto cleanup; + } else { + vshPrint(ctl, "%s", configData); + ret = true; + } + + cleanup: + virshDomainFree(dom); VIR_FREE(xmlData); + VIR_FREE(configData); return ret; } diff --git a/tools/virsh.pod b/tools/virsh.pod index aee964689..049c2f3c7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1433,10 +1433,11 @@ the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>. -=item B<domxml-to-native> I<format> I<xml> +=item B<domxml-to-native> I<format> +{ I<--domain> I<domain-name-or-id-or-uuid> | [I<--xml>] I<xml> } -Convert the file I<xml> in domain XML format to the native guest -configuration format named by I<format>. For QEMU/KVM hypervisor, +Convert the file I<xml> in domain XML format or existing domain to the +native guest configuration format named by I<format>. For QEMU/KVM hypervisor, the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>. -- 2.13.0
On 06/02/2017 11:04 AM, Daniel Liu wrote:
The option allows someone to run domain-to-native on already existing domain without the need of supplying their XML. It is basically wrapper around 'virsh dumpxml | virsh domxml-to-native /dev/stdin'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476 Signed-off-by: Daniel Liu <srwx4096@gmail.com> --- tools/virsh-domain.c | 51 ++++++++++++++++++++++++++++++++++++++------------- tools/virsh.pod | 7 ++++--- 2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ccb514ef9..3496359c5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9848,9 +9848,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} @@ -9859,30 +9863,51 @@ static const vshCmdOptDef opts_domxmltonative[] = { static bool cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) { - bool ret = true; + bool ret = false; const char *format = NULL; - const char *xmlFile = NULL; - char *configData; - char *xmlData; + const char *xml = NULL;
nit: no need to change from xmlFile to xml... The less differences the better ;-)
+ char *xmlData = NULL; + char *configData = NULL;
nit: no need to change order from original, just add ' = NULL' is fine.
unsigned int flags = 0; virshControlPtr priv = ctl->privData; + virDomainPtr dom = NULL;
if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0) + vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0) return false;
- if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &xmlData) < 0) - return false; + VSH_EXCLUSIVE_OPTIONS("domain", "xml");
- configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags); - if (configData != NULL) { - vshPrint(ctl, "%s", configData); - VIR_FREE(configData); + if (vshCommandOptBool(cmd, "domain") && + (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))) + return false; + + if (dom) { + xmlData = virDomainGetXMLDesc(dom, flags); + } else if (xml) { + if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlData) < 0) + goto cleanup; } else { - ret = false; + vshError(ctl, "%s", _("need either domain or domain XML")); + goto cleanup; + } + + if (!xmlData) { + vshError(ctl, "%s", _("failed to retrieve XML")); + goto cleanup; }
+ if (!(configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags))) { + goto cleanup; + } else { + vshPrint(ctl, "%s", configData); + ret = true; + } + + cleanup: + virshDomainFree(dom); VIR_FREE(xmlData); + VIR_FREE(configData); return ret; }
diff --git a/tools/virsh.pod b/tools/virsh.pod index aee964689..049c2f3c7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1433,10 +1433,11 @@ the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
-=item B<domxml-to-native> I<format> I<xml> +=item B<domxml-to-native> I<format> +{ I<--domain> I<domain-name-or-id-or-uuid> | [I<--xml>] I<xml> }
This looks really odd, I think this should be { [I<xml> | [I<--domain> I<domain-name-or-id-uuid>] } I believe that's the way to describe mutually exclusive options where one would be required; otherwise, we have the funky [[ ... ]] which indicates mutually exclusive optional arguments. Search on "{ [" in virsh.pod for some examples. This seems to be the first case where a previously required positional argument can be both a positional and optional as long as the alternative means of describing is provided. Keeping the I<xml> first at least indicates that it can be used *without* the --xml, but is also the "default" when used positionally on the command line. If not used positionally, then the --xml would be required, e.g. if someone used "virsh domxml-to-native --xml @file --format @format". Having the [--domain] indicates that in order to use it, it must have that --domain argument because "virsh domxml-to-native qemu-argv $domainname" would be invalid since the unqualified 2nd argument is expected to be an <xml> file. Whether any of this is written down anywhere - I'm not sure - it's what I've surmised over the few years.
-Convert the file I<xml> in domain XML format to the native guest -configuration format named by I<format>. For QEMU/KVM hypervisor, +Convert the file I<xml> in domain XML format or existing domain to the +native guest configuration format named by I<format>. For QEMU/KVM hypervisor, the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
Convert the file I<xml> into domain XML format or convert an existing I<--domain> to the native guest configuration format named by I<format>. The I<xml> and I<--domain> arguments are mutually exclusive. John
On Wed, Jun 14, 2017 at 08:55:25AM -0400, John Ferlan wrote:
On 06/02/2017 11:04 AM, Daniel Liu wrote:
The option allows someone to run domain-to-native on already existing domain without the need of supplying their XML. It is basically wrapper around 'virsh dumpxml | virsh domxml-to-native /dev/stdin'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476 Signed-off-by: Daniel Liu <srwx4096@gmail.com> --- tools/virsh-domain.c | 51 ++++++++++++++++++++++++++++++++++++++------------- tools/virsh.pod | 7 ++++--- 2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ccb514ef9..3496359c5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9848,9 +9848,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} @@ -9859,30 +9863,51 @@ static const vshCmdOptDef opts_domxmltonative[] = { static bool cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) { - bool ret = true; + bool ret = false; const char *format = NULL; - const char *xmlFile = NULL; - char *configData; - char *xmlData; + const char *xml = NULL;
nit: no need to change from xmlFile to xml... The less differences the better ;-)
There was no change, it is an additional variable, the original one is below. The number of differences would be the same, I believe.
+ char *xmlData = NULL; + char *configData = NULL;
nit: no need to change order from original, just add ' = NULL' is fine.
unsigned int flags = 0; virshControlPtr priv = ctl->privData; + virDomainPtr dom = NULL;
if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0) + vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0) return false;
- if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &xmlData) < 0) - return false; + VSH_EXCLUSIVE_OPTIONS("domain", "xml");
- configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags); - if (configData != NULL) { - vshPrint(ctl, "%s", configData); - VIR_FREE(configData); + if (vshCommandOptBool(cmd, "domain") && + (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))) + return false; + + if (dom) { + xmlData = virDomainGetXMLDesc(dom, flags); + } else if (xml) { + if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlData) < 0) + goto cleanup; } else { - ret = false; + vshError(ctl, "%s", _("need either domain or domain XML")); + goto cleanup; + } + + if (!xmlData) { + vshError(ctl, "%s", _("failed to retrieve XML")); + goto cleanup; }
+ if (!(configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags))) { + goto cleanup; + } else { + vshPrint(ctl, "%s", configData); + ret = true; + } + + cleanup: + virshDomainFree(dom); VIR_FREE(xmlData); + VIR_FREE(configData); return ret; }
diff --git a/tools/virsh.pod b/tools/virsh.pod index aee964689..049c2f3c7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1433,10 +1433,11 @@ the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
-=item B<domxml-to-native> I<format> I<xml> +=item B<domxml-to-native> I<format> +{ I<--domain> I<domain-name-or-id-or-uuid> | [I<--xml>] I<xml> }
This looks really odd, I think this should be
{ [I<xml> | [I<--domain> I<domain-name-or-id-uuid>] }
Actually, the only thing that Dan can change is the order, but the rest should remain. His version shows that you must supply either --domain domain-name-or-id-or-uuid or --xml xml and that in the second case, the --xml is optional. Your suggested solution, however, mean that you must supply either the xml (no mention about the optional --xml) or --domain domain-name-or-id-or-uuid or nothing (in a really weird way), as the latter option is in square brackets. So the only way this could look differently is: { [I<--xml>] I<xml> | I<--domain> I<domain-name-or-id-or-uuid> }
I believe that's the way to describe mutually exclusive options where one would be required; otherwise, we have the funky [[ ... ]] which indicates mutually exclusive optional arguments. Search on "{ [" in virsh.pod for some examples. This seems to be the first case where a previously required positional argument can be both a positional and optional as long as the alternative means of describing is provided.
Keeping the I<xml> first at least indicates that it can be used *without* the --xml, but is also the "default" when used positionally on the command line. If not used positionally, then the --xml would be required, e.g. if someone used "virsh domxml-to-native --xml @file --format @format".
Having the [--domain] indicates that in order to use it, it must have that --domain argument because "virsh domxml-to-native qemu-argv $domainname" would be invalid since the unqualified 2nd argument is expected to be an <xml> file.
Whether any of this is written down anywhere - I'm not sure - it's what I've surmised over the few years.
-Convert the file I<xml> in domain XML format to the native guest -configuration format named by I<format>. For QEMU/KVM hypervisor, +Convert the file I<xml> in domain XML format or existing domain to the +native guest configuration format named by I<format>. For QEMU/KVM hypervisor, the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
Convert the file I<xml> into domain XML format or convert an existing I<--domain> to the native guest configuration format named by I<format>. The I<xml> and I<--domain> arguments are mutually exclusive.
Yes, this sounds better, there is a slight flaw in the original hunk and this explains it more nicely.
John
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 06/14/2017 09:47 AM, Martin Kletzander wrote:
On Wed, Jun 14, 2017 at 08:55:25AM -0400, John Ferlan wrote:
On 06/02/2017 11:04 AM, Daniel Liu wrote:
The option allows someone to run domain-to-native on already existing domain without the need of supplying their XML. It is basically wrapper around 'virsh dumpxml | virsh domxml-to-native /dev/stdin'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476 Signed-off-by: Daniel Liu <srwx4096@gmail.com> --- tools/virsh-domain.c | 51 ++++++++++++++++++++++++++++++++++++++------------- tools/virsh.pod | 7 ++++--- 2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ccb514ef9..3496359c5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9848,9 +9848,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} @@ -9859,30 +9863,51 @@ static const vshCmdOptDef opts_domxmltonative[] = { static bool cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) { - bool ret = true; + bool ret = false; const char *format = NULL; - const char *xmlFile = NULL; - char *configData; - char *xmlData; + const char *xml = NULL;
nit: no need to change from xmlFile to xml... The less differences the better ;-)
There was no change, it is an additional variable, the original one is below. The number of differences would be the same, I believe.
If edit the file and change "xml" to "xmlFile" and change the 3 changed xml variable references things work... Like I said, nit, IDC if it's changed or not...
+ char *xmlData = NULL; + char *configData = NULL;
nit: no need to change order from original, just add ' = NULL' is fine.
unsigned int flags = 0; virshControlPtr priv = ctl->privData; + virDomainPtr dom = NULL;
if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0) + vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0)
e.g., s/&xml/&xmlFile
return false;
- if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &xmlData) < 0) - return false; + VSH_EXCLUSIVE_OPTIONS("domain", "xml");
- configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags); - if (configData != NULL) { - vshPrint(ctl, "%s", configData); - VIR_FREE(configData); + if (vshCommandOptBool(cmd, "domain") && + (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))) + return false; + + if (dom) { + xmlData = virDomainGetXMLDesc(dom, flags); + } else if (xml) {
e.g.: else if (xmlFile) { if (virFileReadAll(xmlFile,...)
+ if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlData) < 0) + goto cleanup; } else { - ret = false; + vshError(ctl, "%s", _("need either domain or domain XML")); + goto cleanup; + } + + if (!xmlData) { + vshError(ctl, "%s", _("failed to retrieve XML")); + goto cleanup; }
+ if (!(configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags))) { + goto cleanup; + } else { + vshPrint(ctl, "%s", configData); + ret = true; + } + + cleanup: + virshDomainFree(dom); VIR_FREE(xmlData); + VIR_FREE(configData); return ret; }
diff --git a/tools/virsh.pod b/tools/virsh.pod index aee964689..049c2f3c7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1433,10 +1433,11 @@ the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
-=item B<domxml-to-native> I<format> I<xml> +=item B<domxml-to-native> I<format> +{ I<--domain> I<domain-name-or-id-or-uuid> | [I<--xml>] I<xml> }
This looks really odd, I think this should be
{ [I<xml> | [I<--domain> I<domain-name-or-id-uuid>] }
Actually, the only thing that Dan can change is the order, but the rest should remain. His version shows that you must supply either --domain domain-name-or-id-or-uuid or --xml xml and that in the second case, the --xml is optional.
Your suggested solution, however, mean that you must supply either the xml (no mention about the optional --xml) or --domain domain-name-or-id-or-uuid or nothing (in a really weird way), as the latter option is in square brackets.
So the only way this could look differently is:
{ [I<--xml>] I<xml> | I<--domain> I<domain-name-or-id-or-uuid> }
That's fine - I considered this too, but the [I<--xml>] looked strange, but I see it's technically correct since having the --xml doesn't necessarily matter and thus enclosed in brackets. John
I believe that's the way to describe mutually exclusive options where one would be required; otherwise, we have the funky [[ ... ]] which indicates mutually exclusive optional arguments. Search on "{ [" in virsh.pod for some examples. This seems to be the first case where a previously required positional argument can be both a positional and optional as long as the alternative means of describing is provided.
Keeping the I<xml> first at least indicates that it can be used *without* the --xml, but is also the "default" when used positionally on the command line. If not used positionally, then the --xml would be required, e.g. if someone used "virsh domxml-to-native --xml @file --format @format".
Having the [--domain] indicates that in order to use it, it must have that --domain argument because "virsh domxml-to-native qemu-argv $domainname" would be invalid since the unqualified 2nd argument is expected to be an <xml> file.
Whether any of this is written down anywhere - I'm not sure - it's what I've surmised over the few years.
-Convert the file I<xml> in domain XML format to the native guest -configuration format named by I<format>. For QEMU/KVM hypervisor, +Convert the file I<xml> in domain XML format or existing domain to the +native guest configuration format named by I<format>. For QEMU/KVM hypervisor, the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
Convert the file I<xml> into domain XML format or convert an existing I<--domain> to the native guest configuration format named by I<format>. The I<xml> and I<--domain> arguments are mutually exclusive.
Yes, this sounds better, there is a slight flaw in the original hunk and this explains it more nicely.
John
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jun 14, 2017 at 10:32:16AM -0400, John Ferlan wrote:
On 06/14/2017 09:47 AM, Martin Kletzander wrote:
On Wed, Jun 14, 2017 at 08:55:25AM -0400, John Ferlan wrote:
On 06/02/2017 11:04 AM, Daniel Liu wrote:
The option allows someone to run domain-to-native on already existing domain without the need of supplying their XML. It is basically wrapper around 'virsh dumpxml | virsh domxml-to-native /dev/stdin'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476 Signed-off-by: Daniel Liu <srwx4096@gmail.com> --- tools/virsh-domain.c | 51 ++++++++++++++++++++++++++++++++++++++------------- tools/virsh.pod | 7 ++++--- 2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ccb514ef9..3496359c5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9848,9 +9848,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} @@ -9859,30 +9863,51 @@ static const vshCmdOptDef opts_domxmltonative[] = { static bool cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) { - bool ret = true; + bool ret = false; const char *format = NULL; - const char *xmlFile = NULL; - char *configData; - char *xmlData; + const char *xml = NULL;
nit: no need to change from xmlFile to xml... The less differences the better ;-)
There was no change, it is an additional variable, the original one is below. The number of differences would be the same, I believe.
If edit the file and change "xml" to "xmlFile" and change the 3 changed xml variable references things work... Like I said, nit, IDC if it's changed or not...
My bad, I misread that, you're right.
[...]
There was no change, it is an additional variable, the original one is below. The number of differences would be the same, I believe.
If edit the file and change "xml" to "xmlFile" and change the 3 changed xml variable references things work... Like I said, nit, IDC if it's changed or not...
My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone? John
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
[...]
There was no change, it is an additional variable, the original one is below. The number of differences would be the same, I believe.
If edit the file and change "xml" to "xmlFile" and change the 3 changed xml variable references things work... Like I said, nit, IDC if it's changed or not...
My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone?
John
WFM Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Hi guys, I updated the source this weekend, I missed the ability of calling help. virsh # domxml-to-native --help NAME domxml-to-native - Convert domain XML to native config SYNOPSIS domxml-to-native <format> [<domain>] [<xml>] DESCRIPTION Convert domain XML config to a native guest configuration format. OPTIONS [--format] <string> target config data type format error: internal error: bad options in command: 'domxml-to-native' This is why: --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = { }, {.name = "domain", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ_OPT, + .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = NULL} VSH_OT_DATA requires VSH_OFLAG_REQ. So, since XML is not required... This diff fits this case. But I'm still confused. Because I cannot check my XML files right now. virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml error: failed to get domain '/home/julio/WINDOWS_7.xml' error: Domain not found: no domain with matching name '/home/julio/WINDOWS_7.xml' 2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
[...]
There was no change, it is an additional variable, the original one is below. The number of differences would be the same, I believe.
If edit the file and change "xml" to "xmlFile" and change the 3 changed xml variable references things work... Like I said, nit, IDC if it's changed or not...
My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone?
John
WFM
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
My apologies. I didn't read the function logic. The second error was introduced by my "fix". The error with help parameter is happening. If --domain is required, I believe that moving to STRING is enough. As I said, VSH_OT_DATA requires VSH_OFLAG_REQ. See vsh.c line 746. {.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") 2017-06-24 17:54 GMT-03:00 Julio Faracco <jcfaracco@gmail.com>:
Hi guys,
I updated the source this weekend, I missed the ability of calling help. virsh # domxml-to-native --help NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [<domain>] [<xml>]
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format error: internal error: bad options in command: 'domxml-to-native'
This is why: --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = { }, {.name = "domain", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ_OPT, + .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = NULL}
VSH_OT_DATA requires VSH_OFLAG_REQ. So, since XML is not required... This diff fits this case. But I'm still confused. Because I cannot check my XML files right now.
virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml error: failed to get domain '/home/julio/WINDOWS_7.xml' error: Domain not found: no domain with matching name '/home/julio/WINDOWS_7.xml'
2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
[...]
>
There was no change, it is an additional variable, the original one is below. The number of differences would be the same, I believe.
If edit the file and change "xml" to "xmlFile" and change the 3 changed xml variable references things work... Like I said, nit, IDC if it's changed or not...
My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone?
John
WFM
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 06/24/2017 08:05 PM, Julio Faracco wrote:
My apologies. I didn't read the function logic. The second error was introduced by my "fix". The error with help parameter is happening.
If --domain is required, I believe that moving to STRING is enough. As I said, VSH_OT_DATA requires VSH_OFLAG_REQ. See vsh.c line 746.
{.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from")
ugh... Could you send a patch please. I do recall at one point wondering what the downside of removing VSH_OFLAG_REQ from "xml" .flags was, no I know. Of course I probably won't remember the next time either Tks, - John
2017-06-24 17:54 GMT-03:00 Julio Faracco <jcfaracco@gmail.com>:
Hi guys,
I updated the source this weekend, I missed the ability of calling help. virsh # domxml-to-native --help NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [<domain>] [<xml>]
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format error: internal error: bad options in command: 'domxml-to-native'
This is why: --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = { }, {.name = "domain", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ_OPT, + .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = NULL}
VSH_OT_DATA requires VSH_OFLAG_REQ. So, since XML is not required... This diff fits this case. But I'm still confused. Because I cannot check my XML files right now.
virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml error: failed to get domain '/home/julio/WINDOWS_7.xml' error: Domain not found: no domain with matching name '/home/julio/WINDOWS_7.xml'
2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
[...]
>> > > There was no change, it is an additional variable, the original one is > below. The number of differences would be the same, I believe. >
If edit the file and change "xml" to "xmlFile" and change the 3 changed xml variable references things work... Like I said, nit, IDC if it's changed or not...
My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone?
John
WFM
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Sun, Jun 25, 2017 at 08:33:39AM -0400, John Ferlan wrote:
On 06/24/2017 08:05 PM, Julio Faracco wrote:
My apologies. I didn't read the function logic. The second error was introduced by my "fix". The error with help parameter is happening.
If --domain is required, I believe that moving to STRING is enough. As I said, VSH_OT_DATA requires VSH_OFLAG_REQ. See vsh.c line 746.
{.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from")
ugh... Could you send a patch please. I do recall at one point wondering what the downside of removing VSH_OFLAG_REQ from "xml" .flags was, no I know. Of course I probably won't remember the next time either
Tks, -
John
Hey John, I attempted to fix it this morning, but I cannot seem to be able to get it fixed the way according to current man page specificiation. Either --help is not working (as you found out), or the command option is not working when I tried to fix it by different ways. Could you help, please or send the patch directly? My understanding based on your reply is that in order to make --help work, there must exist VSH_OFLAG_REQ for --xml. Thanks, Dan
2017-06-24 17:54 GMT-03:00 Julio Faracco <jcfaracco@gmail.com>:
Hi guys,
I updated the source this weekend, I missed the ability of calling help. virsh # domxml-to-native --help NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [<domain>] [<xml>]
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format error: internal error: bad options in command: 'domxml-to-native'
This is why: --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = { }, {.name = "domain", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ_OPT, + .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = NULL}
VSH_OT_DATA requires VSH_OFLAG_REQ. So, since XML is not required... This diff fits this case. But I'm still confused. Because I cannot check my XML files right now.
virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml error: failed to get domain '/home/julio/WINDOWS_7.xml' error: Domain not found: no domain with matching name '/home/julio/WINDOWS_7.xml'
2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
[...]
>>> >> >> There was no change, it is an additional variable, the original one is >> below. The number of differences would be the same, I believe. >> > > If edit the file and change "xml" to "xmlFile" and change the 3 changed > xml variable references things work... Like I said, nit, IDC if it's > changed or not... >
My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone?
John
WFM
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Sun, Jun 25, 2017 at 08:33:39AM -0400, John Ferlan wrote:
On 06/24/2017 08:05 PM, Julio Faracco wrote:
My apologies. I didn't read the function logic. The second error was introduced by my "fix". The error with help parameter is happening.
If --domain is required, I believe that moving to STRING is enough. As I said, VSH_OT_DATA requires VSH_OFLAG_REQ. See vsh.c line 746.
{.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from")
ugh... Could you send a patch please. I do recall at one point wondering what the downside of removing VSH_OFLAG_REQ from "xml" .flags was, no I know. Of course I probably won't remember the next time either
Tks, -
John
The closest I could get so far is by changing this part into the following: {.name = "xml", .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = "domain", .type = VSH_OT_STRING, .help = N_("domain name, id or uuid") }, However, the --help will look like: OPTIONS [--format] <string> target config data type format --xml <string> xml data file to export from --domain <string> domain name, id or uuid , while the requirement is to not make --xml mandatory; i.e.: OPTIONS [--format] <string> target config data type format [--xml] <string> xml data file to export from --domain <string> domain name, id or uuid Dan
2017-06-24 17:54 GMT-03:00 Julio Faracco <jcfaracco@gmail.com>:
Hi guys,
I updated the source this weekend, I missed the ability of calling help. virsh # domxml-to-native --help NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [<domain>] [<xml>]
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format error: internal error: bad options in command: 'domxml-to-native'
This is why: --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = { }, {.name = "domain", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ_OPT, + .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = NULL}
VSH_OT_DATA requires VSH_OFLAG_REQ. So, since XML is not required... This diff fits this case. But I'm still confused. Because I cannot check my XML files right now.
virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml error: failed to get domain '/home/julio/WINDOWS_7.xml' error: Domain not found: no domain with matching name '/home/julio/WINDOWS_7.xml'
2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
[...]
>>> >> >> There was no change, it is an additional variable, the original one is >> below. The number of differences would be the same, I believe. >> > > If edit the file and change "xml" to "xmlFile" and change the 3 changed > xml variable references things work... Like I said, nit, IDC if it's > changed or not... >
My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone?
John
WFM
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Sun, Jun 25, 2017 at 08:33:39AM -0400, John Ferlan wrote:
On 06/24/2017 08:05 PM, Julio Faracco wrote:
My apologies. I didn't read the function logic. The second error was introduced by my "fix". The error with help parameter is happening.
If --domain is required, I believe that moving to STRING is enough. As I said, VSH_OT_DATA requires VSH_OFLAG_REQ. See vsh.c line 746.
{.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from")
ugh... Could you send a patch please. I do recall at one point wondering what the downside of removing VSH_OFLAG_REQ from "xml" .flags was, no I know. Of course I probably won't remember the next time either
Tks, -
John
I think we may need to modify vshCmddefHelp() in vsh.c to really make it look like as expected. For example, in some case, the SYNOPSIS part also displayed incorrectly. SYNOPSIS domxml-to-native <format> <xml> <domain> Instead, the following is similar to what we want: SYNOPSIS- domxml-to-native <format> { [--xml] <xml> | --domain <domain> } Current implementation of vshCmddefHelp() does not handle this "complicated" logic. The question is whether we want to modify it or whether we want to spend the effort modifying it? Dan
2017-06-24 17:54 GMT-03:00 Julio Faracco <jcfaracco@gmail.com>:
Hi guys,
I updated the source this weekend, I missed the ability of calling help. virsh # domxml-to-native --help NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [<domain>] [<xml>]
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format error: internal error: bad options in command: 'domxml-to-native'
This is why: --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = { }, {.name = "domain", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ_OPT, + .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = NULL}
VSH_OT_DATA requires VSH_OFLAG_REQ. So, since XML is not required... This diff fits this case. But I'm still confused. Because I cannot check my XML files right now.
virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml error: failed to get domain '/home/julio/WINDOWS_7.xml' error: Domain not found: no domain with matching name '/home/julio/WINDOWS_7.xml'
2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
[...]
>>> >> >> There was no change, it is an additional variable, the original one is >> below. The number of differences would be the same, I believe. >> > > If edit the file and change "xml" to "xmlFile" and change the 3 changed > xml variable references things work... Like I said, nit, IDC if it's > changed or not... >
My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone?
John
WFM
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi Dan, Have you tested the --xml argument as VSH_OT_ARGV? {.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_ARGV, .help = N_("xml data file to export from") Check the command 'domfsthaw', option 'mountpoint'. This is the best approach that I got until now. 2017-06-26 17:21 GMT-03:00 Dan <srwx4096@gmail.com>:
On Sun, Jun 25, 2017 at 08:33:39AM -0400, John Ferlan wrote:
On 06/24/2017 08:05 PM, Julio Faracco wrote:
My apologies. I didn't read the function logic. The second error was introduced by my "fix". The error with help parameter is happening.
If --domain is required, I believe that moving to STRING is enough. As I said, VSH_OT_DATA requires VSH_OFLAG_REQ. See vsh.c line 746.
{.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from")
ugh... Could you send a patch please. I do recall at one point wondering what the downside of removing VSH_OFLAG_REQ from "xml" .flags was, no I know. Of course I probably won't remember the next time either
Tks, -
John
I think we may need to modify vshCmddefHelp() in vsh.c to really make it look like as expected. For example, in some case, the SYNOPSIS part also displayed incorrectly.
SYNOPSIS domxml-to-native <format> <xml> <domain>
Instead, the following is similar to what we want:
SYNOPSIS- domxml-to-native <format> { [--xml] <xml> | --domain <domain> }
Current implementation of vshCmddefHelp() does not handle this "complicated" logic. The question is whether we want to modify it or whether we want to spend the effort modifying it?
Dan
2017-06-24 17:54 GMT-03:00 Julio Faracco <jcfaracco@gmail.com>:
Hi guys,
I updated the source this weekend, I missed the ability of calling help. virsh # domxml-to-native --help NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [<domain>] [<xml>]
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format error: internal error: bad options in command: 'domxml-to-native'
This is why: --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = { }, {.name = "domain", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ_OPT, + .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = NULL}
VSH_OT_DATA requires VSH_OFLAG_REQ. So, since XML is not required... This diff fits this case. But I'm still confused. Because I cannot check my XML files right now.
virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml error: failed to get domain '/home/julio/WINDOWS_7.xml' error: Domain not found: no domain with matching name '/home/julio/WINDOWS_7.xml'
2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
[...]
>>>> >>> >>> There was no change, it is an additional variable, the original one is >>> below. The number of differences would be the same, I believe. >>> >> >> If edit the file and change "xml" to "xmlFile" and change the 3 changed >> xml variable references things work... Like I said, nit, IDC if it's >> changed or not... >> > > My bad, I misread that, you're right.
In order to "close" on this, if a squash the attach patch does that work for everyone?
John
WFM
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jun 26, 2017 at 09:30:11PM -0300, Julio Faracco wrote:
Hi Dan,
Have you tested the --xml argument as VSH_OT_ARGV? {.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_ARGV, .help = N_("xml data file to export from")
Check the command 'domfsthaw', option 'mountpoint'. This is the best approach that I got until now.
Yes, it is working! Finally I understand it. When an --optionname is optional for a flag, VSH_OT_ARGV should be used. Similarly another such case is schedinfo's --set flag. Yeah, I should be more careful. Thank you very much. Shall I send a new patch using your apprach? Now domxml-to-native --help looks like: NAME domxml-to-native - Convert domain XML to native config SYNOPSIS domxml-to-native <format> [--domain <string>] [[--xml] <string>]... DESCRIPTION Convert domain XML config to a native guest configuration format. OPTIONS [--format] <string> target config data type format --domain <string> domain name, id or uuid [--xml] <string> xml data file to export from Just for aesthetic reason, shall I switch the order of the last two lines? (though I don't see a strong point to do this). Thanks, Dan
2017-06-26 17:21 GMT-03:00 Dan <srwx4096@gmail.com>:
On Sun, Jun 25, 2017 at 08:33:39AM -0400, John Ferlan wrote:
On 06/24/2017 08:05 PM, Julio Faracco wrote:
My apologies. I didn't read the function logic. The second error was introduced by my "fix". The error with help parameter is happening.
If --domain is required, I believe that moving to STRING is enough. As I said, VSH_OT_DATA requires VSH_OFLAG_REQ. See vsh.c line 746.
{.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from")
ugh... Could you send a patch please. I do recall at one point wondering what the downside of removing VSH_OFLAG_REQ from "xml" .flags was, no I know. Of course I probably won't remember the next time either
Tks, -
John
I think we may need to modify vshCmddefHelp() in vsh.c to really make it look like as expected. For example, in some case, the SYNOPSIS part also displayed incorrectly.
SYNOPSIS domxml-to-native <format> <xml> <domain>
Instead, the following is similar to what we want:
SYNOPSIS- domxml-to-native <format> { [--xml] <xml> | --domain <domain> }
Current implementation of vshCmddefHelp() does not handle this "complicated" logic. The question is whether we want to modify it or whether we want to spend the effort modifying it?
Dan
2017-06-24 17:54 GMT-03:00 Julio Faracco <jcfaracco@gmail.com>:
Hi guys,
I updated the source this weekend, I missed the ability of calling help. virsh # domxml-to-native --help NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [<domain>] [<xml>]
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format error: internal error: bad options in command: 'domxml-to-native'
This is why: --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = { }, {.name = "domain", .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ_OPT, + .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .help = N_("xml data file to export from") }, {.name = NULL}
VSH_OT_DATA requires VSH_OFLAG_REQ. So, since XML is not required... This diff fits this case. But I'm still confused. Because I cannot check my XML files right now.
virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml error: failed to get domain '/home/julio/WINDOWS_7.xml' error: Domain not found: no domain with matching name '/home/julio/WINDOWS_7.xml'
2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote: > > > [...] > >>>>> >>>> >>>> There was no change, it is an additional variable, the original one is >>>> below. The number of differences would be the same, I believe. >>>> >>> >>> If edit the file and change "xml" to "xmlFile" and change the 3 changed >>> xml variable references things work... Like I said, nit, IDC if it's >>> changed or not... >>> >> >> My bad, I misread that, you're right. > > > In order to "close" on this, if a squash the attach patch does that work > for everyone? > > John
WFM
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jun 26, 2017 at 09:05:23PM -0400, Dan wrote:
On Mon, Jun 26, 2017 at 09:30:11PM -0300, Julio Faracco wrote:
Hi Dan,
Have you tested the --xml argument as VSH_OT_ARGV? {.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_ARGV, .help = N_("xml data file to export from")
Check the command 'domfsthaw', option 'mountpoint'. This is the best approach that I got until now.
Yes, it is working! Finally I understand it. When an --optionname is optional for a flag, VSH_OT_ARGV should be used. Similarly another such case is schedinfo's --set flag. Yeah, I should be more careful. Thank you very much. Shall I send a new patch using your apprach?
Now domxml-to-native --help looks like:
NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [--domain <string>] [[--xml] <string>]...
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format --domain <string> domain name, id or uuid [--xml] <string> xml data file to export from
Just for aesthetic reason, shall I switch the order of the last two lines? (though I don't see a strong point to do this).
Please, submit a patch for that and don't change the line ordering. Thank you.
On Tue, Jun 27, 2017 at 10:26:32AM +0200, Martin Kletzander wrote:
On Mon, Jun 26, 2017 at 09:05:23PM -0400, Dan wrote:
On Mon, Jun 26, 2017 at 09:30:11PM -0300, Julio Faracco wrote:
Hi Dan,
Have you tested the --xml argument as VSH_OT_ARGV? {.name = "domain", - .type = VSH_OT_DATA, + .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, .help = N_("domain name, id or uuid") }, {.name = "xml", - .type = VSH_OT_DATA, + .type = VSH_OT_ARGV, .help = N_("xml data file to export from")
Check the command 'domfsthaw', option 'mountpoint'. This is the best approach that I got until now.
Yes, it is working! Finally I understand it. When an --optionname is optional for a flag, VSH_OT_ARGV should be used. Similarly another such case is schedinfo's --set flag. Yeah, I should be more careful. Thank you very much. Shall I send a new patch using your apprach?
Now domxml-to-native --help looks like:
NAME domxml-to-native - Convert domain XML to native config
SYNOPSIS domxml-to-native <format> [--domain <string>] [[--xml] <string>]...
DESCRIPTION Convert domain XML config to a native guest configuration format.
OPTIONS [--format] <string> target config data type format --domain <string> domain name, id or uuid [--xml] <string> xml data file to export from
Just for aesthetic reason, shall I switch the order of the last two lines? (though I don't see a strong point to do this).
Please, submit a patch for that and don't change the line ordering. Thank you. Just sent. Please review and let me know if there is any more issue.
Thank you very much, Dan
On Wed, Jun 14, 2017 at 08:55:25AM -0400, John Ferlan wrote:
On 06/02/2017 11:04 AM, Daniel Liu wrote:
The option allows someone to run domain-to-native on already existing domain without the need of supplying their XML. It is basically wrapper around 'virsh dumpxml | virsh domxml-to-native /dev/stdin'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476 Signed-off-by: Daniel Liu <srwx4096@gmail.com> --- tools/virsh-domain.c | 51 ++++++++++++++++++++++++++++++++++++++------------- tools/virsh.pod | 7 ++++--- 2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ccb514ef9..3496359c5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9848,9 +9848,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} @@ -9859,30 +9863,51 @@ static const vshCmdOptDef opts_domxmltonative[] = { static bool cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) { - bool ret = true; + bool ret = false; const char *format = NULL; - const char *xmlFile = NULL; - char *configData; - char *xmlData; + const char *xml = NULL;
nit: no need to change from xmlFile to xml... The less differences the better ;-)
+ char *xmlData = NULL; + char *configData = NULL;
nit: no need to change order from original, just add ' = NULL' is fine.
unsigned int flags = 0; virshControlPtr priv = ctl->privData; + virDomainPtr dom = NULL;
if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0) + vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0) return false;
- if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &xmlData) < 0) - return false; + VSH_EXCLUSIVE_OPTIONS("domain", "xml");
- configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags); - if (configData != NULL) { - vshPrint(ctl, "%s", configData); - VIR_FREE(configData); + if (vshCommandOptBool(cmd, "domain") && + (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))) + return false; + + if (dom) { + xmlData = virDomainGetXMLDesc(dom, flags); + } else if (xml) { + if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlData) < 0) + goto cleanup; } else { - ret = false; + vshError(ctl, "%s", _("need either domain or domain XML")); + goto cleanup; + } + + if (!xmlData) { + vshError(ctl, "%s", _("failed to retrieve XML")); + goto cleanup; }
+ if (!(configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, flags))) { + goto cleanup; + } else { + vshPrint(ctl, "%s", configData); + ret = true; + } + + cleanup: + virshDomainFree(dom); VIR_FREE(xmlData); + VIR_FREE(configData); return ret; }
diff --git a/tools/virsh.pod b/tools/virsh.pod index aee964689..049c2f3c7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1433,10 +1433,11 @@ the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
-=item B<domxml-to-native> I<format> I<xml> +=item B<domxml-to-native> I<format> +{ I<--domain> I<domain-name-or-id-or-uuid> | [I<--xml>] I<xml> }
This looks really odd, I think this should be
{ [I<xml> | [I<--domain> I<domain-name-or-id-uuid>] }
I believe that's the way to describe mutually exclusive options where one would be required; otherwise, we have the funky [[ ... ]] which indicates mutually exclusive optional arguments. Search on "{ [" in virsh.pod for some examples. This seems to be the first case where a previously required positional argument can be both a positional and optional as long as the alternative means of describing is provided.
Keeping the I<xml> first at least indicates that it can be used *without* the --xml, but is also the "default" when used positionally on the command line. If not used positionally, then the --xml would be required, e.g. if someone used "virsh domxml-to-native --xml @file --format @format".
Having the [--domain] indicates that in order to use it, it must have that --domain argument because "virsh domxml-to-native qemu-argv $domainname" would be invalid since the unqualified 2nd argument is expected to be an <xml> file.
Whether any of this is written down anywhere - I'm not sure - it's what I've surmised over the few years.
-Convert the file I<xml> in domain XML format to the native guest -configuration format named by I<format>. For QEMU/KVM hypervisor, +Convert the file I<xml> in domain XML format or existing domain to the +native guest configuration format named by I<format>. For QEMU/KVM hypervisor, the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
Convert the file I<xml> into domain XML format or convert an existing I<--domain> to the native guest configuration format named by I<format>. The I<xml> and I<--domain> arguments are mutually exclusive.
John
I found the man page is still missing. Shall I send a patch for the documentation? Dan
[...]
-Convert the file I<xml> in domain XML format to the native guest -configuration format named by I<format>. For QEMU/KVM hypervisor, +Convert the file I<xml> in domain XML format or existing domain to the +native guest configuration format named by I<format>. For QEMU/KVM hypervisor, the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the I<format> argument may be B<xen-xm>, B<xen-xl>, or B<xen-sxpr>. For LXC hypervisor, the I<format> argument must be B<lxc-tools>.
Convert the file I<xml> into domain XML format or convert an existing I<--domain> to the native guest configuration format named by I<format>. The I<xml> and I<--domain> arguments are mutually exclusive.
John I found the man page is still missing. Shall I send a patch for the documentation?
Dan
What do you mean by missing? Commit id '41eb92783' updated the virsh.pod which is used to generate the man page. After a build in your branch, use "man tools/virsh.1" to see the change. If you mean installed version - I recall there being some sort of strangeness using make install vs. a real RPM install from your branch, but I don't recall exactly what that issue was... John
participants (5)
-
Dan -
Daniel Liu
-
John Ferlan -
Julio Faracco -
Martin Kletzander