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

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