Re: [libvirt] [PATCH v4] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

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(a)gmail.com&gt;: > 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(a)gmail.com&gt;: >> >> 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(a)redhat.com&gt;: >> >>> 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(a)redhat.com&gt; >> >>> >> >>> -- >> >>> libvir-list mailing list >> >>> libvir-list(a)redhat.com >> >>> https://www.redhat.com/mailman/listinfo/libvir-list