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